From 0eb31eb8e327c40369a6b88410107ccacc5698dc Mon Sep 17 00:00:00 2001 From: LukeParkerDev <10430890+Hona@users.noreply.github.com> Date: Thu, 2 Jul 2026 15:48:10 +1000 Subject: [PATCH] fix(app): resolve session lineage across in-place session changes --- packages/app/src/pages/session.tsx | 44 +---- .../app/src/pages/session/session-lineage.ts | 62 +++++++ .../app/test-browser/session-lineage.test.ts | 168 ++++++++++++++++++ 3 files changed, 236 insertions(+), 38 deletions(-) create mode 100644 packages/app/src/pages/session/session-lineage.ts create mode 100644 packages/app/test-browser/session-lineage.test.ts diff --git a/packages/app/src/pages/session.tsx b/packages/app/src/pages/session.tsx index 899490b8fa..379a007f31 100644 --- a/packages/app/src/pages/session.tsx +++ b/packages/app/src/pages/session.tsx @@ -11,7 +11,6 @@ import { createMemo, createEffect, createComputed, - createSignal, on, onMount, type ParentProps, @@ -90,6 +89,7 @@ import { legacySessionHref, requireServerKey, sessionHref } from "@/utils/sessio 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 } type FollowupEdit = Pick @@ -228,8 +228,12 @@ function ResolvedTargetSessionRoute() { const params = useParams<{ serverKey: string; id: string }>() const settings = useSettings() const tabs = useTabs() + const sync = useServerSync() const serverKey = createMemo(() => requireServerKey(params.serverKey)) - const current = createSessionLineage(() => params.id) + const current = createSessionLineage( + () => params.id, + () => sync().session.lineage, + ) const directory = createMemo(() => current()?.session.directory) const targetDirectory = () => directory()! @@ -260,42 +264,6 @@ function ResolvedTargetSessionRoute() { ) } -// Reactive session lineage for the target session route, read from the sync store. -// The route keys its consumer to the session ID, so resolution runs once per target. -// Resolution is imperative rather than a resource on purpose: a resource created here -// would be created inside the router's navigation transition, and suspending that -// transition deadlocks the URL commit and double-mounts the session header portals -// from the transition's shadow render. `lineage.resolve` fills the sync store, which -// the returned accessor observes; resolve failures rethrow on read so the enclosing -// SessionRouteErrorBoundary renders the scoped session error. -function createSessionLineage(sessionID: () => string) { - const sync = useServerSync() - const cached = createMemo(() => sync().session.lineage.peek(sessionID())) - const [failure, setFailure] = createSignal() - const [settled, setSettled] = createSignal(false) - onMount(() => { - if (cached()) { - setSettled(true) - return - } - sync() - .session.lineage.resolve(sessionID()) - .then(() => setSettled(true)) - .catch((error) => setFailure(() => error)) - }) - return createMemo(() => { - const error = failure() - if (error) throw error - const lineage = cached() - // The viewed session is pinned and pinned lineages are exempt from cache pruning, - // so a lineage missing after settlement means the session (or an ancestor) was - // deleted, possibly by another client. Match the resolve error so the boundary - // shows the session not found fallback. - if (!lineage && settled()) throw new Error(`Session not found: ${sessionID()}`) - return lineage - }) -} - function TargetSessionPage() { const sdk = useSDK() const serverSDK = useServerSDK() diff --git a/packages/app/src/pages/session/session-lineage.ts b/packages/app/src/pages/session/session-lineage.ts new file mode 100644 index 0000000000..97416a9152 --- /dev/null +++ b/packages/app/src/pages/session/session-lineage.ts @@ -0,0 +1,62 @@ +import { createEffect, createMemo, createSignal, on, onCleanup } from "solid-js" + +// Reactive session lineage for the target session route, read from the sync store. +// All session tabs on a server share one route instance, so the target session ID +// changes in place; the effect is only a trigger that starts resolution for the +// current target, and each run cancels the previous one through onCleanup so a +// late result from an abandoned target is dropped. Resolution is imperative rather +// than a resource on purpose: a resource created here would be created inside the +// router's navigation transition, and suspending that transition deadlocks the URL +// commit and double-mounts the session header portals from the transition's shadow +// render. +// +// The returned accessor is a pure derivation. The sync cache is authoritative, and +// status only applies while it matches the current target: on navigation the memo +// re-evaluates before the trigger runs, so trusting a previous target's settlement +// would fabricate a not-found for a session that simply has not resolved yet. +// Resolve failures rethrow on read so the enclosing SessionRouteErrorBoundary +// renders the scoped session error. +export function createSessionLineage( + sessionID: () => string, + lineage: () => { peek: (id: string) => T | undefined; resolve: (id: string) => Promise }, +) { + const cached = createMemo(() => lineage().peek(sessionID())) + const [status, setStatus] = createSignal<{ id: string; settled: boolean; failure?: unknown }>() + + createEffect( + on(sessionID, (id) => { + let stale = false + onCleanup(() => { + stale = true + }) + if (cached()) { + setStatus({ id, settled: true }) + return + } + setStatus({ id, settled: false }) + lineage() + .resolve(id) + .then(() => { + if (!stale) setStatus({ id, settled: true }) + }) + .catch((error) => { + if (!stale) setStatus({ id, settled: true, failure: error }) + }) + }), + ) + + return createMemo(() => { + const id = sessionID() + const value = cached() + if (value) return value + const state = status() + if (state?.id !== id) return undefined + if (state.failure !== undefined) throw state.failure + // The viewed session is pinned and pinned lineages are exempt from cache pruning, + // so a lineage missing after settlement means the session (or an ancestor) was + // deleted, possibly by another client. Match the resolve error so the boundary + // shows the session not found fallback. + if (state.settled) throw new Error(`Session not found: ${id}`) + return undefined + }) +} diff --git a/packages/app/test-browser/session-lineage.test.ts b/packages/app/test-browser/session-lineage.test.ts new file mode 100644 index 0000000000..70443d3d9f --- /dev/null +++ b/packages/app/test-browser/session-lineage.test.ts @@ -0,0 +1,168 @@ +import { expect, test } from "bun:test" +import { createRoot, createSignal } from "solid-js" +import { createSessionLineage } from "@/pages/session/session-lineage" + +type Lineage = { session: { id: string; directory: string } } + +// Fake sync lineage store: peek reads a reactive cache, resolve returns a +// deferred promise the test settles or fails explicitly. The lineage memo is +// live (read below), so it recomputes eagerly on cache/status writes — throws +// surface at the write site, which is also where the enclosing ErrorBoundary +// would see them in the app. Assertions wrap write + read to cover both. +function createFixture(initial: Record = {}) { + const [cache, setCache] = createSignal(initial) + const deferred = new Map>() + const resolves: string[] = [] + return { + resolves, + lineage: { + peek: (id: string) => cache()[id], + resolve: (id: string) => { + resolves.push(id) + const entry = deferred.get(id) ?? Promise.withResolvers() + deferred.set(id, entry) + return entry.promise + }, + }, + settle(id: string) { + setCache({ ...cache(), [id]: { session: { id, directory: `/dir/${id}` } } }) + deferred.get(id)?.resolve(undefined) + }, + fail(id: string, error: unknown) { + deferred.get(id)?.reject(error) + }, + remove(id: string) { + const next = { ...cache() } + delete next[id] + setCache(next) + }, + } +} + +const flush = async () => { + await Promise.resolve() + await Promise.resolve() +} + +const lineageOf = (id: string): Lineage => ({ session: { id, directory: `/dir/${id}` } }) + +test("resolves an uncached session and exposes its lineage", async () => { + await createRoot(async (dispose) => { + const fixture = createFixture() + const current = createSessionLineage( + () => "ses_a", + () => fixture.lineage, + ) + + expect(current()).toBeUndefined() + await flush() + expect(fixture.resolves).toEqual(["ses_a"]) + + fixture.settle("ses_a") + await flush() + expect(current()?.session.id).toBe("ses_a") + + dispose() + }) +}) + +// Session tabs on the same server share one route instance, so navigating to +// another session changes the id in place; resolution must follow it instead +// of reporting the new session as missing. +test("re-resolves when navigating to an uncached session without a remount", async () => { + await createRoot(async (dispose) => { + const fixture = createFixture({ ses_a: lineageOf("ses_a") }) + const [id, setId] = createSignal("ses_a") + const current = createSessionLineage(id, () => fixture.lineage) + + await flush() + expect(current()?.session.id).toBe("ses_a") + + expect(() => { + setId("ses_b") + current() + }).not.toThrow() + expect(fixture.resolves).toEqual(["ses_b"]) + + fixture.settle("ses_b") + await flush() + expect(current()?.session.id).toBe("ses_b") + + dispose() + }) +}) + +// A late failure from a session the user already navigated away from must not +// poison the currently viewed session. +test("ignores a stale resolution failure after the target changes", async () => { + await createRoot(async (dispose) => { + const fixture = createFixture() + const [id, setId] = createSignal("ses_a") + const current = createSessionLineage(id, () => fixture.lineage) + + await flush() + setId("ses_b") + fixture.fail("ses_a", new Error("Session not found: ses_a")) + await flush() + + expect(() => current()).not.toThrow() + fixture.settle("ses_b") + await flush() + expect(current()?.session.id).toBe("ses_b") + + dispose() + }) +}) + +test("returning to a pruned session re-resolves instead of throwing not found", async () => { + await createRoot(async (dispose) => { + const fixture = createFixture() + const [id, setId] = createSignal("ses_a") + const current = createSessionLineage(id, () => fixture.lineage) + + await flush() + fixture.settle("ses_a") + await flush() + + setId("ses_b") + fixture.settle("ses_b") + await flush() + + fixture.remove("ses_a") + expect(() => { + setId("ses_a") + current() + }).not.toThrow() + expect(fixture.resolves).toEqual(["ses_a", "ses_b", "ses_a"]) + + fixture.settle("ses_a") + await flush() + expect(current()?.session.id).toBe("ses_a") + + dispose() + }) +}) + +// The viewed session is pinned in the cache, so disappearing after settlement +// means it was deleted; the boundary must show the not found fallback. +test("throws not found when the settled session is deleted", async () => { + await createRoot(async (dispose) => { + const fixture = createFixture() + const current = createSessionLineage( + () => "ses_a", + () => fixture.lineage, + ) + + await flush() + fixture.settle("ses_a") + await flush() + expect(current()?.session.id).toBe("ses_a") + + expect(() => { + fixture.remove("ses_a") + current() + }).toThrow("Session not found: ses_a") + + dispose() + }) +})