feat(web): open the monitoring column on a failed connection (#1621)
On a connection failure the user was left without an obvious way to see why. Open the monitoring column automatically on the transition into the `error` status (mirroring the connect-success behavior from #1616) so the diagnostics surface right when they're needed. - Broaden the auto-open effect to fire on the transition into `error` as well as `connected`, tracked via a general previous-status ref so it fires once per transition and never on a mount that starts errored. - A failed connect never negotiated capabilities, so the failure column offers History (the local client-side message log) and — for non-stdio transports — Network (HTTP requests), the entries that explain the failure. Logs is omitted (it needs the server `logging` capability). - `effectivePinned` now allows the column while `error`, not only while `connected`; it still closes on a clean disconnect or narrowing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
This commit is contained in:
parent
89e9a275ef
commit
85153c6232
2 changed files with 125 additions and 23 deletions
|
|
@ -1376,6 +1376,86 @@ describe("InspectorView", () => {
|
|||
).toBeNull();
|
||||
});
|
||||
|
||||
// A failed connect leaves the errored server's id set but no
|
||||
// initializeResult (capabilities were never negotiated).
|
||||
function failedHttp(overrides: Partial<InspectorViewProps> = {}) {
|
||||
return makeProps({
|
||||
servers: [httpServer],
|
||||
activeServer: "beta",
|
||||
connectionStatus: "error",
|
||||
initializeResult: undefined,
|
||||
...overrides,
|
||||
});
|
||||
}
|
||||
|
||||
it("opens the monitoring column on a failed connection attempt (#1621)", async () => {
|
||||
monitorWide.value = true;
|
||||
const { rerender } = renderWithMantine(
|
||||
<StatefulInspectorViewHost
|
||||
{...makeProps({
|
||||
servers: [httpServer],
|
||||
activeServer: undefined,
|
||||
connectionStatus: "disconnected",
|
||||
})}
|
||||
/>,
|
||||
);
|
||||
// Disconnected: the column is closed.
|
||||
expect(
|
||||
screen.queryByRole("button", { name: "Close monitoring column" }),
|
||||
).toBeNull();
|
||||
|
||||
// A connection failure (→ error) opens it to surface the diagnostics.
|
||||
rerender(<StatefulInspectorViewHost {...failedHttp()} />);
|
||||
expect(
|
||||
await screen.findByRole("button", { name: "Close monitoring column" }),
|
||||
).toBeInTheDocument();
|
||||
|
||||
// The failure column offers the History + Network diagnostics, but not
|
||||
// Logs (a failed connect never negotiated the logging capability).
|
||||
expect(screen.getByRole("radio", { name: "History" })).toBeChecked();
|
||||
expect(
|
||||
screen.getByRole("radio", { name: "Network" }),
|
||||
).toBeInTheDocument();
|
||||
expect(screen.queryByRole("radio", { name: "Logs" })).toBeNull();
|
||||
});
|
||||
|
||||
it("omits Network from the failure column for a stdio server (#1621)", async () => {
|
||||
monitorWide.value = true;
|
||||
const stdioErr: ServerEntry = {
|
||||
id: "beta",
|
||||
name: "Beta",
|
||||
config: { type: "stdio", command: "missing-bin" },
|
||||
connection: { status: "disconnected" },
|
||||
};
|
||||
const { rerender } = renderWithMantine(
|
||||
<StatefulInspectorViewHost
|
||||
{...makeProps({
|
||||
servers: [stdioErr],
|
||||
activeServer: undefined,
|
||||
connectionStatus: "disconnected",
|
||||
})}
|
||||
/>,
|
||||
);
|
||||
rerender(
|
||||
<StatefulInspectorViewHost {...failedHttp({ servers: [stdioErr] })} />,
|
||||
);
|
||||
// stdio has no HTTP traffic, so the failure column shows History only.
|
||||
expect(
|
||||
await screen.findByRole("radio", { name: "History" }),
|
||||
).toBeInTheDocument();
|
||||
expect(screen.queryByRole("radio", { name: "Network" })).toBeNull();
|
||||
});
|
||||
|
||||
it("does not auto-open on a mount that starts already errored (#1621)", () => {
|
||||
// No transition into error, and no stored preference, so the column stays
|
||||
// closed — a user who closed it isn't fought on remount.
|
||||
monitorWide.value = true;
|
||||
renderWithMantine(<StatefulInspectorViewHost {...failedHttp()} />);
|
||||
expect(
|
||||
screen.queryByRole("button", { name: "Close monitoring column" }),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("pins the monitor group into the column and removes it from the header", async () => {
|
||||
monitorWide.value = true;
|
||||
renderWithMantine(<StatefulInspectorViewHost {...connectedHttp()} />);
|
||||
|
|
|
|||
|
|
@ -654,21 +654,26 @@ export function InspectorView({
|
|||
getInitialValueInEffect: false,
|
||||
});
|
||||
|
||||
// Open the monitoring column when a connection is established (#1616). Gated on
|
||||
// the disconnected → connected *transition* (via the ref) rather than the
|
||||
// "connected" state itself, so it fires on an actual connect — not on every
|
||||
// render while connected, and not on a mount that starts already-connected
|
||||
// (which would fight a user who closed it). 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, since this
|
||||
// Open the monitoring column when a connection is established (#1616) OR when a
|
||||
// connection 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.
|
||||
const wasConnectedRef = useRef(connectionStatus === "connected");
|
||||
const prevStatusRef = useRef(connectionStatus);
|
||||
useEffect(() => {
|
||||
const isConnected = connectionStatus === "connected";
|
||||
if (isConnected && !wasConnectedRef.current) {
|
||||
const prev = prevStatusRef.current;
|
||||
prevStatusRef.current = connectionStatus;
|
||||
const becameConnected =
|
||||
connectionStatus === "connected" && prev !== "connected";
|
||||
const becameError = connectionStatus === "error" && prev !== "error";
|
||||
if (becameConnected || becameError) {
|
||||
setMonitorPinned(true);
|
||||
}
|
||||
wasConnectedRef.current = isConnected;
|
||||
}, [connectionStatus, setMonitorPinned]);
|
||||
|
||||
const appTools = useMemo<Tool[]>(() => {
|
||||
|
|
@ -740,20 +745,37 @@ export function InspectorView({
|
|||
appTools,
|
||||
]);
|
||||
|
||||
// Monitoring column, derived (#1616). The monitor group is pinned into the
|
||||
// right column only when: the user asked for it, the viewport is wide enough,
|
||||
// a server is connected, and at least one monitor tab is actually available
|
||||
// (capability/stdio aware). Narrowing, disconnecting, or losing the last
|
||||
// monitor capability flips `effectivePinned` false and closes the column —
|
||||
// WITHOUT clearing `monitorPinned`, so it re-opens when the condition returns.
|
||||
// Only the column's close button writes `monitorPinned = false`.
|
||||
// Monitoring column, derived (#1616, #1621). The monitor group is pinned into
|
||||
// the right column only when: the user asked for it, the viewport is wide
|
||||
// enough, the session is connected OR a connect attempt failed, and at least
|
||||
// one monitor tab is actually available (capability/stdio aware). Narrowing,
|
||||
// returning to a clean disconnect, or losing the last monitor capability flips
|
||||
// `effectivePinned` false and closes the column — WITHOUT clearing
|
||||
// `monitorPinned`, so it re-opens when the condition returns. Only the column's
|
||||
// close button writes `monitorPinned = false`.
|
||||
const connected = connectionStatus === "connected";
|
||||
const monitorAvailable = useMemo<string[]>(
|
||||
() => availableTabs.filter((t) => MONITOR_TABS.includes(t)),
|
||||
[availableTabs],
|
||||
);
|
||||
// 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";
|
||||
const monitorAvailable = useMemo<string[]>(() => {
|
||||
if (connected) return availableTabs.filter((t) => MONITOR_TABS.includes(t));
|
||||
if (failed) {
|
||||
// A failed connect never negotiated capabilities, so Logs (gated on the
|
||||
// server's `logging` capability) isn't meaningful. History (the local
|
||||
// client-side message log) always captured the attempt, and Network (HTTP
|
||||
// requests) did too for non-stdio transports — those are what explain the
|
||||
// failure, so offer exactly them.
|
||||
const active = serversInput.find((s) => s.id === activeServer);
|
||||
const isStdio = active ? getServerType(active.config) === "stdio" : false;
|
||||
return isStdio ? [HISTORY_TAB] : [HISTORY_TAB, NETWORK_TAB];
|
||||
}
|
||||
return [];
|
||||
}, [connected, failed, availableTabs, serversInput, activeServer]);
|
||||
const effectivePinned =
|
||||
monitorPinned && !!isWide && connected && monitorAvailable.length > 0;
|
||||
monitorPinned &&
|
||||
!!isWide &&
|
||||
(connected || failed) &&
|
||||
monitorAvailable.length > 0;
|
||||
|
||||
// The header loses the monitor group while the column is open (its screens
|
||||
// live in the column instead); otherwise it shows every available tab.
|
||||
|
|
|
|||
Loading…
Reference in a new issue