Skip to content

Commit b040213

Browse files
fix(frontend): make disconnect indicator clickable to reconnect (#9410)
## 📝 Summary Closes #9385 **This pull request was primarily authored by a coding agent.** ### Problem When marimo is accessed over an SSH-forwarded tunnel (or any flaky link), the kernel WebSocket drops on tunnel reset or laptop sleep. The browser shows the broken-link indicator in the top-left, and the only recovery is a full page reload — disruptive because it resets scroll position, cell focus, and any unsaved client state, despite the kernel surviving the WS drop in edit mode and the server already supporting session resume on a fresh connection (`marimo/_server/api/endpoints/ws/ws_session_connector.py`). The frontend was forcing the reload because of two compounding issues: 1. **One-shot reconnect gate**: `useMarimoKernelConnection.shouldTryReconnecting` only resets on a successful `onOpen`. After partysocket's 10-retry budget is exhausted, the gate stays closed and `ws.reconnect()` is never called again from our code. 2. **Upstream partysocket bug**: in `partysocket@1.1.10`, `_connectLock` leaks in the `_connect()` max-retries early-return — even if the gate were reset, `ws.reconnect()` would silently no-op because the lock is permanently held. Fixed upstream in [partykit#322](cloudflare/partykit#322), released as partysocket 1.1.13. 3. **Stuck spinner**: the `onClose` default branch always sets state to `CONNECTING`, even after partysocket has exhausted its retry budget — the spinner outlives the actual retry loop with no escape. The broken-link icon also gives the visual impression of being clickable but is a no-op `<div>` today. ### What changed This is two commits — a dependency bump (precondition) followed by the frontend fix. #### `chore(frontend): bump partysocket 1.1.10 → 1.1.13` Patch-version bump pulling in the upstream `_connectLock` fix. No API-surface changes — verified that `retryCount`, `reconnect()`, and `maxRetries` signatures are identical between 1.1.10 and 1.1.13. The lockfile diff is exactly 6 lines, all about partysocket. #### `fix(frontend): make disconnect indicator clickable to reconnect` - **Disconnect indicator becomes a real `<button>`** (`status.tsx`). Native button gives Enter/Space activation and accessibility; previously it was a `<div>` with a tooltip. When the connection is in a recoverable CLOSED state, clicking calls a new `reconnect()` exposed by `useMarimoKernelConnection`. When the connection is `CLOSED` for any other reason, the button is `disabled`. - **`reconnect()` probes the runtime first** via the existing `runtimeManager.isHealthy()` (`/health`). If the server is genuinely unreachable (e.g. the SSH tunnel is dead), we transition straight back to `CLOSED + KERNEL_DISCONNECTED` in ~1s instead of burning partysocket's full retry budget on the silent click. The probe also resets the one-shot reconnect gate so subsequent automatic retries work. - **Surface `CLOSED` when partysocket has given up.** In the `onClose` default branch, when `ws.retryCount >= MAX_RETRIES`, transition to `CLOSED + KERNEL_DISCONNECTED` instead of `CONNECTING`. This makes the disconnect icon reappear and become clickable again, so the user can manually retry without reloading. Without this, the spinner stayed up forever even after partysocket had stopped trying. - **Refactor**: the inline switch on `event.reason` in `onClose` is extracted into a pure `classifyCloseEvent` helper (`close-handler.ts`) so the regression behavior can be unit-tested. The hook is now thin glue: classify → setConnection → optionally close transport or call `tryReconnecting`. - **Tests**: `close-handler.test.ts` (15 cases) covers each terminal reason plus the retry-budget-exhaustion path. `status.test.tsx` (3 cases) covers the click → callback wiring and the disabled state. ### Heads-up for review - **`runtimeManager.isHealthy()` has a side effect** (`setDOMBaseUri()` on success). It was previously only triggered during initial startup; now it can fire on each manual reconnect. Almost certainly a no-op in practice (URL hasn't changed) but flagging in case you want it gated. - **`IConnectionTransport.retryCount` was added** to the transport interface so the close handler can detect partysocket give-up. This leaks a partysocket-specific concept into a transport-agnostic interface; an alternative would be to have the transport emit a synthetic close event with a "gave-up" reason. Open to that direction if preferred — current approach was chosen for minimal diff. - **No automatic-behavior changes** for users on a healthy connection. The only paths that exercise the new code are user-initiated clicks and the previously-stuck post-give-up case. - **No new user-facing strings.** `"kernel not found"` / tooltip text reuse existing copy; only new string is the tooltip suffix `"— click to reconnect"`. ## 📋 Pre-Review Checklist <!-- These checks need to be completed before a PR is reviewed --> - [ ] For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on [Discord](https://marimo.io/discord?ref=pr), or the community [discussions](https://github.com/marimo-team/marimo/discussions) (Please provide a link if applicable). - [x] Any AI generated code has been reviewed line-by-line by the human PR author, who stands by it. - [x] Video or media evidence is provided for any visual changes (optional). <!-- PR is more likely to be merged if evidence is provided for changes made --> ## ✅ Merge Checklist - [x] I have read the [contributor guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md). - [x] Documentation has been updated where applicable, including docstrings for API changes. - [x] Tests have been added for the changes made. I have read the CLA Document and I hereby sign the CLA --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent 3e688b3 commit b040213

13 files changed

Lines changed: 599 additions & 76 deletions

File tree

frontend/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@
120120
"lz-string": "^1.5.0",
121121
"marked": "^15.0.12",
122122
"mermaid": "^11.12.3",
123-
"partysocket": "1.1.10",
123+
"partysocket": "1.1.13",
124124
"path-to-regexp": "^8.4.0",
125125
"plotly.js": "^3.3.1",
126126
"pyodide": "0.27.7",

frontend/src/components/editor/app-container.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,26 @@ interface Props {
1515
connection: ConnectionStatus;
1616
isRunning: boolean;
1717
width: AppConfig["width"];
18+
onReconnect?: () => void;
1819
}
1920

2021
export const AppContainer: React.FC<PropsWithChildren<Props>> = ({
2122
width,
2223
connection,
2324
isRunning,
2425
children,
26+
onReconnect,
2527
}) => {
2628
const connectionState = connection.state;
2729

2830
return (
2931
<>
3032
<DynamicFavicon isRunning={isRunning} />
31-
<StatusOverlay connection={connection} isRunning={isRunning} />
33+
<StatusOverlay
34+
connection={connection}
35+
isRunning={isRunning}
36+
onReconnect={onReconnect}
37+
/>
3238
<PyodideLoader>
3339
<WrappedWithSidebar>
3440
{/** oxlint-ignore-next-line -- ID is used by other components to grab the DOM element */}
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/* Copyright 2026 Marimo. All rights reserved. */
2+
// @vitest-environment jsdom
3+
4+
import { fireEvent, render } from "@testing-library/react";
5+
import { createStore, Provider as JotaiProvider } from "jotai";
6+
import type React from "react";
7+
import { describe, expect, it, vi } from "vitest";
8+
import { TooltipProvider } from "@/components/ui/tooltip";
9+
import { viewStateAtom } from "@/core/mode";
10+
import {
11+
type ConnectionStatus,
12+
WebSocketClosedReason,
13+
WebSocketState,
14+
} from "@/core/websocket/types";
15+
import { StatusOverlay } from "../status";
16+
17+
function renderOverlay(
18+
connection: ConnectionStatus,
19+
onReconnect?: () => void,
20+
): ReturnType<typeof render> {
21+
const store = createStore();
22+
store.set(viewStateAtom, { mode: "edit", cellAnchor: null });
23+
const wrapper: React.FC<React.PropsWithChildren> = ({ children }) => (
24+
<JotaiProvider store={store}>
25+
<TooltipProvider>{children}</TooltipProvider>
26+
</JotaiProvider>
27+
);
28+
return render(
29+
<StatusOverlay
30+
connection={connection}
31+
isRunning={false}
32+
onReconnect={onReconnect}
33+
/>,
34+
{ wrapper },
35+
);
36+
}
37+
38+
describe("StatusOverlay disconnect indicator", () => {
39+
it("invokes onReconnect when the disconnect icon is clicked", () => {
40+
const onReconnect = vi.fn();
41+
const { getByTestId } = renderOverlay(
42+
{
43+
state: WebSocketState.CLOSED,
44+
code: WebSocketClosedReason.KERNEL_DISCONNECTED,
45+
reason: "kernel not found",
46+
},
47+
onReconnect,
48+
);
49+
50+
const icon = getByTestId("disconnected-indicator") as HTMLButtonElement;
51+
expect(icon.tagName).toBe("BUTTON");
52+
expect(icon.disabled).toBe(false);
53+
expect(icon.getAttribute("aria-label")).toBe("Reconnect to app");
54+
fireEvent.click(icon);
55+
expect(onReconnect).toHaveBeenCalledTimes(1);
56+
});
57+
58+
it("renders a disabled button when no onReconnect is provided", () => {
59+
const { getByTestId } = renderOverlay({
60+
state: WebSocketState.CLOSED,
61+
code: WebSocketClosedReason.KERNEL_DISCONNECTED,
62+
reason: "kernel not found",
63+
});
64+
65+
const button = getByTestId("disconnected-indicator");
66+
expect((button as HTMLButtonElement).disabled).toBe(true);
67+
});
68+
69+
it.each([
70+
[
71+
WebSocketClosedReason.MALFORMED_QUERY,
72+
"the kernel did not recognize a request; please file a bug with marimo",
73+
],
74+
[
75+
WebSocketClosedReason.KERNEL_STARTUP_ERROR,
76+
"Failed to start kernel sandbox",
77+
],
78+
])(
79+
"renders a disabled button for non-recoverable close reason %s",
80+
(code, reason) => {
81+
const onReconnect = vi.fn();
82+
const { getByTestId } = renderOverlay(
83+
{ state: WebSocketState.CLOSED, code, reason },
84+
onReconnect,
85+
);
86+
87+
const button = getByTestId("disconnected-indicator") as HTMLButtonElement;
88+
expect(button.disabled).toBe(true);
89+
fireEvent.click(button);
90+
expect(onReconnect).not.toHaveBeenCalled();
91+
},
92+
);
93+
94+
it("does not render the disconnect icon when another tab has taken over", () => {
95+
const onReconnect = vi.fn();
96+
const { queryByTestId } = renderOverlay(
97+
{
98+
state: WebSocketState.CLOSED,
99+
code: WebSocketClosedReason.ALREADY_RUNNING,
100+
reason: "another browser tab is already connected to the kernel",
101+
canTakeover: true,
102+
},
103+
onReconnect,
104+
);
105+
106+
expect(queryByTestId("disconnected-indicator")).toBeNull();
107+
});
108+
});

frontend/src/components/editor/header/status.tsx

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,26 @@ import { Tooltip } from "@/components/ui/tooltip";
77
import { notebookScrollToRunning } from "@/core/cells/actions";
88
import { onlyScratchpadIsRunningAtom } from "@/core/cells/cells";
99
import { viewStateAtom } from "@/core/mode";
10-
import { type ConnectionStatus, WebSocketState } from "@/core/websocket/types";
10+
import {
11+
type ConnectionStatus,
12+
WebSocketClosedReason,
13+
WebSocketState,
14+
} from "@/core/websocket/types";
1115
import { cn } from "@/utils/cn";
1216

1317
export const StatusOverlay: React.FC<{
1418
connection: ConnectionStatus;
1519
isRunning: boolean;
16-
}> = ({ connection, isRunning }) => {
20+
onReconnect?: () => void;
21+
}> = ({ connection, isRunning, onReconnect }) => {
1722
const { mode } = useAtomValue(viewStateAtom);
1823
const isClosed = connection.state === WebSocketState.CLOSED;
1924
const isOpen = connection.state === WebSocketState.OPEN;
25+
// Only KERNEL_DISCONNECTED is recoverable by a retry. Other terminal
26+
// reasons (MALFORMED_QUERY, KERNEL_STARTUP_ERROR) would deterministically
27+
// fail the same way; ALREADY_RUNNING is handled by `LockedIcon` below.
28+
const canReconnect =
29+
isClosed && connection.code === WebSocketClosedReason.KERNEL_DISCONNECTED;
2030

2131
return (
2232
<>
@@ -28,7 +38,11 @@ export const StatusOverlay: React.FC<{
2838
)}
2939
>
3040
{isOpen && isRunning && <RunningIcon />}
31-
{isClosed && !connection.canTakeover && <DisconnectedIcon />}
41+
{isClosed && !connection.canTakeover && (
42+
<DisconnectedIcon
43+
onReconnect={canReconnect ? onReconnect : undefined}
44+
/>
45+
)}
3246
{isClosed && connection.canTakeover && <LockedIcon />}
3347
</div>
3448
</>
@@ -37,13 +51,33 @@ export const StatusOverlay: React.FC<{
3751

3852
const topLeftStatus = "print:hidden pointer-events-auto hover:cursor-pointer";
3953

40-
const DisconnectedIcon = () => (
41-
<Tooltip content="App disconnected">
42-
<div className={topLeftStatus}>
43-
<UnlinkIcon className="w-[25px] h-[25px] text-(--red-11)" />
44-
</div>
45-
</Tooltip>
46-
);
54+
const DisconnectedIcon: React.FC<{ onReconnect?: () => void }> = ({
55+
onReconnect,
56+
}) => {
57+
const disabled = !onReconnect;
58+
return (
59+
<Tooltip
60+
content={
61+
disabled ? "App disconnected" : "App disconnected — click to reconnect"
62+
}
63+
>
64+
{/* Wrapper span keeps the tooltip reachable when the button is
65+
disabled — a disabled <button> swallows pointer events. */}
66+
<span tabIndex={disabled ? 0 : -1}>
67+
<button
68+
type="button"
69+
className={cn(topLeftStatus, "bg-transparent border-0 p-0")}
70+
aria-label={disabled ? "App disconnected" : "Reconnect to app"}
71+
data-testid="disconnected-indicator"
72+
onClick={onReconnect}
73+
disabled={disabled}
74+
>
75+
<UnlinkIcon className="w-[25px] h-[25px] text-(--red-11)" />
76+
</button>
77+
</span>
78+
</Tooltip>
79+
);
80+
};
4781

4882
const LockedIcon = () => (
4983
<Tooltip content="Notebook locked">

frontend/src/core/edit-app.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ export const EditApp: React.FC<AppProps> = ({
7979
};
8080
}, []);
8181

82-
const { connection } = useMarimoKernelConnection({
82+
const { connection, reconnect } = useMarimoKernelConnection({
8383
autoInstantiate: userConfig.runtime.auto_instantiate,
8484
setCells: (cells, layout) => {
8585
setCells(cells);
@@ -147,6 +147,7 @@ export const EditApp: React.FC<AppProps> = ({
147147
connection={connection}
148148
isRunning={isRunning}
149149
width={appConfig.width}
150+
onReconnect={reconnect}
150151
>
151152
<AppHeader
152153
connection={connection}

frontend/src/core/run-app.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export const RunApp: React.FC<AppProps> = ({ appConfig }) => {
3838
};
3939
}, []);
4040

41-
const { connection } = useMarimoKernelConnection({
41+
const { connection, reconnect } = useMarimoKernelConnection({
4242
autoInstantiate: true,
4343
setCells: setCells,
4444
sessionId: getSessionId(),
@@ -84,6 +84,7 @@ export const RunApp: React.FC<AppProps> = ({ appConfig }) => {
8484
connection={connection}
8585
isRunning={isRunning}
8686
width={appConfig.width}
87+
onReconnect={reconnect}
8788
>
8889
<AppHeader connection={connection} className="sm:pt-8">
8990
{galleryHref && (

0 commit comments

Comments
 (0)