diff --git a/packages/app/src/pages/session.tsx b/packages/app/src/pages/session.tsx index b2b897c62b..d6ef37c5fa 100644 --- a/packages/app/src/pages/session.tsx +++ b/packages/app/src/pages/session.tsx @@ -89,6 +89,7 @@ import { formatServerError, isSessionNotFoundError } from "@/utils/server-errors import { legacySessionHref, requireServerKey, sessionHref } from "@/utils/session-route" import { useUsageExceededDialogs } from "./session/usage-exceeded-dialogs" import { createSessionOwnership } from "./session/session-ownership" +import { SessionRouteBoundary } from "./session/route-boundary" type FollowupItem = FollowupDraft & { id: string } type FollowupEdit = Pick @@ -146,34 +147,38 @@ export function SessionPage() { export function TargetSessionRoute() { const params = useParams<{ serverKey: string; id: string }>() return ( - - - - - + ( + + )} + > + + ) } -function SessionRouteErrorBoundary( - props: ParentProps<{ sessionID?: string; serverKey?: ServerConnection.Key; padded?: boolean }>, -) { +function SessionRouteFallback(props: { + error: unknown + sessionID?: string + serverKey?: ServerConnection.Key + padded?: boolean +}) { const settings = useSettings() return ( - - settings.general.newLayoutDesigns() ? ( - - - - - - ) : ( - - ) - } - > - {props.children} - + }> + + + + + + ) } diff --git a/packages/app/src/pages/session/route-boundary.ts b/packages/app/src/pages/session/route-boundary.ts new file mode 100644 index 0000000000..9df8d8e8c2 --- /dev/null +++ b/packages/app/src/pages/session/route-boundary.ts @@ -0,0 +1,37 @@ +import { ErrorBoundary, Show, createComponent, createEffect, on } from "solid-js" +import type { JSX } from "solid-js" + +// Structural boundary for the target session route: decides when the route +// subtree remounts and how route-level errors are scoped. Kept free of app +// contexts (and JSX) so the remount semantics can be tested directly. +// +// Keyed by server only. Workspace-scoped state (notably TerminalProvider and +// its PTY WebSockets) lives inside the route subtree, so switching session +// tabs within the same workspace must not remount it; session changes are +// handled reactively below (TargetSessionPage re-keys per workspace). +export function SessionRouteBoundary(props: { + serverKey: string | undefined + sessionID: string | undefined + fallback: (error: unknown) => JSX.Element + children: JSX.Element +}) { + return createComponent(Show, { + get when() { + return props.serverKey + }, + keyed: true, + get children() { + return createComponent(ErrorBoundary, { + fallback: (error: unknown, reset: () => void) => { + // Without a per-session remount, a stale error (e.g. session not + // found) must clear when navigating to a different session. + createEffect(on(() => props.sessionID, reset, { defer: true })) + return props.fallback(error) + }, + get children() { + return props.children + }, + }) + }, + }) +} diff --git a/packages/app/test-browser/session-route-boundary.test.ts b/packages/app/test-browser/session-route-boundary.test.ts new file mode 100644 index 0000000000..13f35d43dc --- /dev/null +++ b/packages/app/test-browser/session-route-boundary.test.ts @@ -0,0 +1,85 @@ +import { expect, test } from "bun:test" +import { createComponent, createSignal, onCleanup } from "solid-js" +import { render } from "solid-js/web" +import { SessionRouteBoundary } from "@/pages/session/route-boundary" + +// Terminals (and other workspace-scoped state) live inside the route subtree, +// so switching session tabs within the same server must not remount it. +test("switching sessions on the same server does not remount the route subtree", () => { + const [server, setServer] = createSignal("srv_1") + const [session, setSession] = createSignal("ses_a") + let mounts = 0 + let disposals = 0 + const Probe = () => { + mounts += 1 + onCleanup(() => { + disposals += 1 + }) + return null + } + + const dispose = render( + () => + createComponent(SessionRouteBoundary, { + get serverKey() { + return server() + }, + get sessionID() { + return session() + }, + fallback: () => null, + get children() { + return createComponent(Probe, {}) + }, + }), + document.createElement("div"), + ) + + const initialMounts = mounts + expect(initialMounts).toBeGreaterThan(0) + + setSession("ses_b") + expect(mounts).toBe(initialMounts) + expect(disposals).toBe(0) + + setServer("srv_2") + expect(mounts).toBeGreaterThan(initialMounts) + expect(disposals).toBeGreaterThan(0) + + dispose() +}) + +// Without a per-session remount, the error boundary must clear a stale error +// (e.g. session not found) when navigating to a different session. +test("route error clears when navigating to a different session", () => { + const [session, setSession] = createSignal("ses_a") + const [broken, setBroken] = createSignal(true) + const Thrower = () => { + if (broken()) throw new Error(`Session not found: ${session()}`) + return "content" + } + + const container = document.createElement("div") + const dispose = render( + () => + createComponent(SessionRouteBoundary, { + serverKey: "srv_1", + get sessionID() { + return session() + }, + fallback: () => "error-fallback", + get children() { + return createComponent(Thrower, {}) + }, + }), + container, + ) + + expect(container.textContent).toBe("error-fallback") + + setBroken(false) + setSession("ses_b") + expect(container.textContent).toBe("content") + + dispose() +})