From cecdc0948570207e47ef9f18a1aa01928c10962f Mon Sep 17 00:00:00 2001 From: Brendan Allan Date: Thu, 2 Jul 2026 15:19:23 +0800 Subject: [PATCH] simplify session keying --- packages/app/src/pages/session.tsx | 55 +++++++------ .../app/src/pages/session/route-boundary.ts | 28 ------- .../session-route-boundary.test.ts | 78 ------------------- 3 files changed, 33 insertions(+), 128 deletions(-) delete mode 100644 packages/app/src/pages/session/route-boundary.ts delete mode 100644 packages/app/test-browser/session-route-boundary.test.ts diff --git a/packages/app/src/pages/session.tsx b/packages/app/src/pages/session.tsx index c558df6ceb..f154e8f377 100644 --- a/packages/app/src/pages/session.tsx +++ b/packages/app/src/pages/session.tsx @@ -88,7 +88,6 @@ import { formatServerError, isLocalSessionNotFoundError, isSessionNotFoundError import { legacySessionHref, requireServerKey, sessionHref } from "@/utils/session-route" import { useUsageExceededDialogs } from "./session/usage-exceeded-dialogs" import { createSessionOwnership } from "./session/session-ownership" -import { SessionRouteErrorBoundary } from "./session/route-boundary" import { createSessionLineage } from "./session/session-lineage" type FollowupItem = FollowupDraft & { id: string } @@ -147,27 +146,32 @@ export function SessionPage() { export function TargetSessionRouteContent() { const params = useParams<{ serverKey: string; id: string }>() return ( - ( - - )} - > + ) } -function SessionRouteFallback(props: { error: unknown; sessionID: string; serverKey: ServerConnection.Key }) { +function SessionRouteErrorBoundary( + props: ParentProps<{ sessionID?: string; serverKey?: ServerConnection.Key; padded?: boolean }>, +) { const settings = useSettings() return ( - }> - - - - - - + + settings.general.newLayoutDesigns() ? ( + + + + + + ) : ( + + ) + } + > + {props.children} + ) } @@ -382,6 +386,7 @@ export default function Page() { }) const workspaceTabs = createMemo(() => layout.tabs(workspaceKey)) + const sessionPanelKey = createMemo(() => (params.id ? `${serverSDK().scope}\0${params.id}` : undefined)) createEffect( on( @@ -2013,13 +2018,19 @@ export default function Page() { width: sessionPanelWidth(), }} > - - {settings.general.newLayoutDesigns() ? ( - {sessionPanelContent()} - ) : ( - sessionPanelContent() - )} - + {settings.general.newLayoutDesigns() ? ( + + {(_) => ( + + {sessionPanelContent()} + + )} + + ) : ( + + {sessionPanelContent()} + + )}
size.start()}> diff --git a/packages/app/src/pages/session/route-boundary.ts b/packages/app/src/pages/session/route-boundary.ts deleted file mode 100644 index ad852685d5..0000000000 --- a/packages/app/src/pages/session/route-boundary.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { ErrorBoundary, createComponent, createEffect, on } from "solid-js" -import type { JSX } from "solid-js" - -// Error scope for the target session route. All session tabs on a server share -// one route instance, so this must NOT key or remount per session: the subtree -// holds workspace-scoped state (notably TerminalProvider and its PTY -// WebSockets) that has to survive switching tabs within the same workspace. -// Remount boundaries live elsewhere: app.tsx keys the route per server around -// the server-scoped providers, and TargetSessionPage re-keys per workspace. -// Context-free so these semantics are directly unit-testable, and JSX-free so -// bun test can import it without the Solid JSX transform. -export function SessionRouteErrorBoundary(props: { - sessionID: string - fallback: (error: unknown) => JSX.Element - children: JSX.Element -}) { - return createComponent(ErrorBoundary, { - fallback: (error: unknown, reset: () => void) => { - // A stale error (e.g. session not found) must clear when navigating to a - // different session tab; mirrors the panel boundary reset inside Page. - 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 deleted file mode 100644 index bd7aaa0d38..0000000000 --- a/packages/app/test-browser/session-route-boundary.test.ts +++ /dev/null @@ -1,78 +0,0 @@ -import { expect, test } from "bun:test" -import { createComponent, createSignal, onCleanup } from "solid-js" -import { render } from "solid-js/web" -import { SessionRouteErrorBoundary } from "@/pages/session/route-boundary" - -// All session tabs on a server share one route instance, and the subtree holds -// workspace-scoped state (notably the terminal and its PTY WebSockets), so -// switching session tabs must not remount it. Remounting is owned elsewhere: -// per server in app.tsx and per workspace in TargetSessionPage. -test("switching sessions does not remount the route subtree", () => { - 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(SessionRouteErrorBoundary, { - 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) - - 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(SessionRouteErrorBoundary, { - 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() -})