simplify session keying

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 (
-    <SessionRouteErrorBoundary
-      sessionID={params.id}
-      fallback={(error) => (
-        <SessionRouteFallback error={error} sessionID={params.id} serverKey={requireServerKey(params.serverKey)} />
-      )}
-    >
+    <SessionRouteErrorBoundary sessionID={params.id} serverKey={requireServerKey(params.serverKey)} padded>
       <ResolvedTargetSessionRoute />
     </SessionRouteErrorBoundary>
   )
 }
 
-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 (
-    <Show when={settings.general.newLayoutDesigns()} fallback={<ErrorPage error={props.error} />}>
-      <SessionRouteFrame padded>
-        <SessionPanelFrame newLayout raised>
-          <SessionErrorFallback error={props.error} sessionID={props.sessionID} serverKey={props.serverKey} />
-        </SessionPanelFrame>
-      </SessionRouteFrame>
-    </Show>
+    <ErrorBoundary
+      fallback={(error) =>
+        settings.general.newLayoutDesigns() ? (
+          <SessionRouteFrame padded={props.padded}>
+            <SessionPanelFrame newLayout raised={!!props.sessionID}>
+              <SessionErrorFallback error={error} sessionID={props.sessionID} serverKey={props.serverKey} />
+            </SessionPanelFrame>
+          </SessionRouteFrame>
+        ) : (
+          <ErrorPage error={error} />
+        )
+      }
+    >
+      {props.children}
+    </ErrorBoundary>
   )
 }
 
@@ -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(),
           }}
         >
-          <SessionPanelFrame newLayout={settings.general.newLayoutDesigns()} raised={!!params.id}>
-            {settings.general.newLayoutDesigns() ? (
-              <ErrorBoundary fallback={sessionErrorFallback}>{sessionPanelContent()}</ErrorBoundary>
-            ) : (
-              sessionPanelContent()
-            )}
-          </SessionPanelFrame>
+          {settings.general.newLayoutDesigns() ? (
+            <Show when={sessionPanelKey()} keyed>
+              {(_) => (
+                <SessionPanelFrame newLayout raised={!!params.id}>
+                  <ErrorBoundary fallback={sessionErrorFallback}>{sessionPanelContent()}</ErrorBoundary>
+                </SessionPanelFrame>
+              )}
+            </Show>
+          ) : (
+            <SessionPanelFrame newLayout={false} raised={!!params.id}>
+              {sessionPanelContent()}
+            </SessionPanelFrame>
+          )}
 
           <Show when={desktopReviewOpen()}>
             <div onPointerDown={() => size.start()}>

packages/app/src/pages/session/route-boundary.ts

@@ -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
-    },
-  })
-}

packages/app/test-browser/session-route-boundary.test.ts

@@ -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()
-})