refactor(app): drop redundant server keying from session route boundary

This commit is contained in:
LukeParkerDev 2026-07-02 14:57:44 +10:00
commit a09f6a87c3
3 changed files with 27 additions and 45 deletions

View file

@ -89,7 +89,7 @@ import { formatServerError, isSessionNotFoundError } from "@/utils/server-errors
import { legacySessionHref, requireServerKey, sessionHref } from "@/utils/session-route" import { legacySessionHref, requireServerKey, sessionHref } from "@/utils/session-route"
import { useUsageExceededDialogs } from "./session/usage-exceeded-dialogs" import { useUsageExceededDialogs } from "./session/usage-exceeded-dialogs"
import { createSessionOwnership } from "./session/session-ownership" import { createSessionOwnership } from "./session/session-ownership"
import { SessionRouteBoundary } from "./session/route-boundary" import { SessionRouteErrorBoundary } from "./session/route-boundary"
type FollowupItem = FollowupDraft & { id: string } type FollowupItem = FollowupDraft & { id: string }
type FollowupEdit = Pick<FollowupItem, "id" | "prompt" | "context"> type FollowupEdit = Pick<FollowupItem, "id" | "prompt" | "context">
@ -147,8 +147,7 @@ export function SessionPage() {
export function TargetSessionRoute() { export function TargetSessionRoute() {
const params = useParams<{ serverKey: string; id: string }>() const params = useParams<{ serverKey: string; id: string }>()
return ( return (
<SessionRouteBoundary <SessionRouteErrorBoundary
serverKey={params.serverKey}
sessionID={params.id} sessionID={params.id}
fallback={(error) => ( fallback={(error) => (
<SessionRouteFallback <SessionRouteFallback
@ -160,7 +159,7 @@ export function TargetSessionRoute() {
)} )}
> >
<ResolvedTargetSessionRoute /> <ResolvedTargetSessionRoute />
</SessionRouteBoundary> </SessionRouteErrorBoundary>
) )
} }

View file

@ -1,37 +1,27 @@
import { ErrorBoundary, Show, createComponent, createEffect, on } from "solid-js" import { ErrorBoundary, createComponent, createEffect, on } from "solid-js"
import type { JSX } from "solid-js" import type { JSX } from "solid-js"
// Structural boundary for the target session route: decides when the route // Error scope for the target session route. All session tabs on a server share
// subtree remounts and how route-level errors are scoped. Kept free of app // one route instance, so this must NOT key or remount per session: the subtree
// contexts (and JSX) so the remount semantics can be tested directly. // holds workspace-scoped state (notably TerminalProvider and its PTY
// // WebSockets) that has to survive switching tabs within the same workspace.
// Keyed by server only. Workspace-scoped state (notably TerminalProvider and // Remount boundaries live elsewhere: app.tsx keys the route per server around
// its PTY WebSockets) lives inside the route subtree, so switching session // the server-scoped providers, and TargetSessionPage re-keys per workspace.
// tabs within the same workspace must not remount it; session changes are // Kept free of app contexts (and JSX) so these semantics are directly testable.
// handled reactively below (TargetSessionPage re-keys per workspace). export function SessionRouteErrorBoundary(props: {
export function SessionRouteBoundary(props: {
serverKey: string | undefined
sessionID: string | undefined sessionID: string | undefined
fallback: (error: unknown) => JSX.Element fallback: (error: unknown) => JSX.Element
children: JSX.Element children: JSX.Element
}) { }) {
return createComponent(Show, { return createComponent(ErrorBoundary, {
get when() { fallback: (error: unknown, reset: () => void) => {
return props.serverKey // 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)
}, },
keyed: true,
get children() { get children() {
return createComponent(ErrorBoundary, { return props.children
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
},
})
}, },
}) })
} }

View file

@ -1,12 +1,13 @@
import { expect, test } from "bun:test" import { expect, test } from "bun:test"
import { createComponent, createSignal, onCleanup } from "solid-js" import { createComponent, createSignal, onCleanup } from "solid-js"
import { render } from "solid-js/web" import { render } from "solid-js/web"
import { SessionRouteBoundary } from "@/pages/session/route-boundary" import { SessionRouteErrorBoundary } from "@/pages/session/route-boundary"
// Terminals (and other workspace-scoped state) live inside the route subtree, // All session tabs on a server share one route instance, and the subtree holds
// so switching session tabs within the same server must not remount it. // workspace-scoped state (notably the terminal and its PTY WebSockets), so
test("switching sessions on the same server does not remount the route subtree", () => { // switching session tabs must not remount it. Remounting is owned elsewhere:
const [server, setServer] = createSignal("srv_1") // 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") const [session, setSession] = createSignal("ses_a")
let mounts = 0 let mounts = 0
let disposals = 0 let disposals = 0
@ -20,10 +21,7 @@ test("switching sessions on the same server does not remount the route subtree",
const dispose = render( const dispose = render(
() => () =>
createComponent(SessionRouteBoundary, { createComponent(SessionRouteErrorBoundary, {
get serverKey() {
return server()
},
get sessionID() { get sessionID() {
return session() return session()
}, },
@ -42,10 +40,6 @@ test("switching sessions on the same server does not remount the route subtree",
expect(mounts).toBe(initialMounts) expect(mounts).toBe(initialMounts)
expect(disposals).toBe(0) expect(disposals).toBe(0)
setServer("srv_2")
expect(mounts).toBeGreaterThan(initialMounts)
expect(disposals).toBeGreaterThan(0)
dispose() dispose()
}) })
@ -62,8 +56,7 @@ test("route error clears when navigating to a different session", () => {
const container = document.createElement("div") const container = document.createElement("div")
const dispose = render( const dispose = render(
() => () =>
createComponent(SessionRouteBoundary, { createComponent(SessionRouteErrorBoundary, {
serverKey: "srv_1",
get sessionID() { get sessionID() {
return session() return session()
}, },