From 9bef3c9cb3bf975ba002ef8cdd5c6f449c7b139d Mon Sep 17 00:00:00 2001 From: LukeParkerDev <10430890+Hona@users.noreply.github.com> Date: Thu, 13 Aug 2026 13:20:29 +1000 Subject: [PATCH] fix(app): close client lifecycle gaps --- .../regression/session-request-docks.spec.ts | 1 - .../app/src/context/server-session.test.ts | 80 ++++++++++-- packages/app/src/context/server-session.ts | 48 ++++--- packages/app/src/context/server-sync.tsx | 118 +++++++++++++----- packages/app/src/pages/directory-layout.tsx | 4 +- .../pages/home/home-sessions-controller.tsx | 2 +- packages/app/src/pages/session.tsx | 18 +-- .../composer/session-question-dock.tsx | 12 +- .../session/v2/session-file-browser-tab.tsx | 6 +- 9 files changed, 207 insertions(+), 82 deletions(-) diff --git a/packages/app/e2e/regression/session-request-docks.spec.ts b/packages/app/e2e/regression/session-request-docks.spec.ts index cd03afe45a..5feb6670a3 100644 --- a/packages/app/e2e/regression/session-request-docks.spec.ts +++ b/packages/app/e2e/regression/session-request-docks.spec.ts @@ -32,7 +32,6 @@ test("shows a pending question dock", async ({ page }) => { ], }, ], - sessionStatus: { [sessionID]: { type: "busy" } }, }) await page.goto(`/${base64Encode(directory)}/session/${sessionID}`) diff --git a/packages/app/src/context/server-session.test.ts b/packages/app/src/context/server-session.test.ts index 3d4e9ba271..c3121caf97 100644 --- a/packages/app/src/context/server-session.test.ts +++ b/packages/app/src/context/server-session.test.ts @@ -398,7 +398,18 @@ describe("server session", () => { test("does not let transient hydration overwrite newer events", async () => { const ctx = setup({ child: session("child") }) const response = Promise.withResolvers<{ pending: SessionInboxInfo[]; forms: FormInfo[] }>() - const hydration = ctx.store.hydrateTransient("child", () => response.promise) + const form: FormInfo = { + id: "frm_1", + sessionID: "child", + title: "Choose", + fields: [{ key: "choice", type: "string" as const, title: "Choice" }], + } + let loads = 0 + const hydration = ctx.store.hydrateTransient("child", () => { + loads += 1 + if (loads === 1) return response.promise + return Promise.resolve({ pending: ctx.store.data.pending.child ?? [], forms: [form] }) + }) ctx.store.applyV2({ id: "evt_admitted", @@ -412,22 +423,16 @@ describe("server session", () => { } as OpenCodeEvent) response.resolve({ pending: [], - forms: [ - { - id: "frm_1", - sessionID: "child", - title: "Choose", - fields: [{ key: "choice", type: "string", title: "Choice" }], - }, - ], + forms: [form], }) await hydration expect(ctx.store.data.pending.child).toMatchObject([{ id: "msg_input" }]) expect(ctx.store.data.form.child).toMatchObject([{ id: "frm_1" }]) + expect(loads).toBe(2) }) - test("removes compaction inbox state when compaction settles", () => { + test("removes only the compaction input named by the start event", () => { const ctx = setup({ child: session("child") }) const apply = (input: object) => ctx.store.applyV2(input as OpenCodeEvent) apply({ @@ -445,13 +450,32 @@ describe("server session", () => { expect(ctx.store.data.pending.child).toHaveLength(1) apply({ - id: "evt_compaction_ended", + id: "evt_compaction_inbox_2", created: 2, + type: "session.inbox.enqueued", + data: { + sessionID: "child", + inboxID: "msg_compaction_2", + item: { type: "compaction", delivery: "queue", payload: { reason: "manual" } }, + }, + }) + apply({ + id: "evt_compaction_ended", + created: 3, type: "session.compaction.ended", data: { sessionID: "child", reason: "manual", text: "summary", recent: "recent" }, }) - expect(ctx.store.data.pending.child).toEqual([]) + expect(ctx.store.data.pending.child).toHaveLength(2) + + apply({ + id: "evt_compaction_started", + created: 4, + type: "session.compaction.started", + data: { sessionID: "child", inputID: "msg_compaction", reason: "manual" }, + }) + + expect(ctx.store.data.pending.child?.map((item) => item.id)).toEqual(["msg_compaction_2"]) }) test("projects committed revert before server reconciliation", () => { @@ -475,6 +499,38 @@ describe("server session", () => { expect(ctx.store.data.session_message.child?.map((message) => message.id)).toEqual(["msg_1"]) }) + test("does not restore a message hydrated before a committed revert", async () => { + const response = Promise.withResolvers() + const store = createServerSession({ + session: { + get: async () => session("child"), + message: () => response.promise, + } as unknown as SessionApi, + message: { + list: async () => currentPage({ data: [], response: { headers: new Headers() } }), + } as unknown as MessageApi, + }) + store.remember(session("child")) + + store.applyV2({ + id: "evt_delivered", + created: 1, + type: "session.inbox.delivered", + data: { sessionID: "child", inboxID: "msg_2" }, + } as OpenCodeEvent) + store.applyV2({ + id: "evt_revert", + created: 2, + type: "session.revert.committed", + data: { sessionID: "child", to: "msg_2" }, + } as OpenCodeEvent) + response.resolve({ id: "msg_2", type: "user", text: "stale", time: { created: 1 } }) + await response.promise + await Bun.sleep(0) + + expect(store.data.session_message.child ?? []).toEqual([]) + }) + test("resolves lineage by session ID without directory", async () => { const ctx = setup({ child: session("child", "root"), root: session("root") }) diff --git a/packages/app/src/context/server-session.ts b/packages/app/src/context/server-session.ts index 5eaf8690ed..1139abcdad 100644 --- a/packages/app/src/context/server-session.ts +++ b/packages/app/src/context/server-session.ts @@ -217,6 +217,7 @@ export function createServerSession( const v2 = createV2SessionReducer() const pendingRevision = new Map() const formRevision = new Map() + const messageHydrationRevision = new Map() const invalidated = new Set() let invalidationRevision = 0 const messageLoads = new Map() @@ -481,6 +482,7 @@ export function createServerSession( if (evicted.has(item.sessionID)) deltaBases.delete(partID) } sessionIDs.forEach((sessionID) => { + messageHydrationRevision.set(sessionID, (messageHydrationRevision.get(sessionID) ?? 0) + 1) generations.delete(sessionID) clearOptimistic(sessionID) requests.delete(sessionID) @@ -910,9 +912,14 @@ export function createServerSession( const hydrateV2Message = (sessionID: string, messageID: string) => { if (!sessionApi) return + const active = generation(sessionID) + const revision = messageHydrationRevision.get(sessionID) ?? 0 void sessionApi .message({ sessionID, messageID }) .then((message) => { + if (generations.get(sessionID) !== active) return + if ((messageHydrationRevision.get(sessionID) ?? 0) !== revision) return + if (removedMessages.get(sessionID)?.has(message.id)) return const current = data.session_message[sessionID] ?? [] const messages = [...current.filter((item) => item.id !== message.id), message].sort(compareMessages) projectV2({ sessionID, messages, touched: [message.id] }) @@ -952,7 +959,6 @@ export function createServerSession( event.type === "session.inbox.cancelled" || event.type === "session.inbox.delivered" || event.type === "session.compaction.started" || - event.type === "session.compaction.ended" || event.type === "session.compaction.failed" ) pendingRevision.set(sessionID, (pendingRevision.get(sessionID) ?? 0) + 1) @@ -980,13 +986,6 @@ export function createServerSession( setData("pending", sessionID, (items) => items?.filter((item) => item.id !== event.data.inputID)) setData("input", sessionID, (items) => items?.filter((id) => id !== event.data.inputID)) } - if (event.type === "session.compaction.ended") { - const compactions = new Set( - data.pending[sessionID]?.filter((item) => item.type === "compaction").map((item) => item.id), - ) - setData("pending", sessionID, (items) => items?.filter((item) => item.type !== "compaction")) - setData("input", sessionID, (items) => items?.filter((id) => !compactions.has(id))) - } const info = data.info[sessionID] const reduction = v2.reduce(data.session_message[sessionID] ?? [], event, info) if (reduction) { @@ -999,7 +998,9 @@ export function createServerSession( if (info) remember({ ...info, model: event.data.model }) if (data.session_message[sessionID]) hydrateV2Message(sessionID, event.id.replace(/^evt_/, "msg_")) } - if (event.type === "session.renamed") + if (event.type === "session.renamed" && info) + remember({ ...info, title: event.data.title, time: { ...info.time, updated: event.created } }) + if (event.type === "session.renamed" && !info) void resolve(sessionID) .then((current) => remember({ ...current, title: event.data.title, time: { ...current.time, updated: event.created } }), @@ -1037,10 +1038,12 @@ export function createServerSession( if (event.type === "session.revert.staged" && info) remember({ ...info, revert: event.data.revert }) if (event.type === "session.revert.cleared" && info) remember({ ...info, revert: undefined }) if (event.type === "session.revert.committed") { + messageHydrationRevision.set(sessionID, (messageHydrationRevision.get(sessionID) ?? 0) + 1) if (info) remember({ ...info, revert: undefined }) setData("input", sessionID, (items) => items?.filter((id) => id < event.data.to)) const source = data.session_message[sessionID] ?? [] const removed = source.filter((message) => message.id >= event.data.to).map((message) => message.id) + removedMessages.set(sessionID, new Set([...(removedMessages.get(sessionID) ?? []), ...removed])) projectV2({ sessionID, messages: source.filter((message) => message.id < event.data.to), @@ -1387,18 +1390,23 @@ export function createServerSession( sessionID: string, load: () => Promise<{ pending: SessionInboxInfo[]; forms: FormInfo[] }>, ) { - const pendingAt = pendingRevision.get(sessionID) ?? 0 - const formAt = formRevision.get(sessionID) ?? 0 - const result = await load() - if ((pendingRevision.get(sessionID) ?? 0) === pendingAt) { - setData("pending", sessionID, reconcile(result.pending)) - setData( - "input", - sessionID, - reconcile(result.pending.filter((item) => item.type !== "compaction").map((item) => item.id)), - ) + while (true) { + const pendingAt = pendingRevision.get(sessionID) ?? 0 + const formAt = formRevision.get(sessionID) ?? 0 + const result = await load() + const pendingStable = (pendingRevision.get(sessionID) ?? 0) === pendingAt + const formStable = (formRevision.get(sessionID) ?? 0) === formAt + if (pendingStable) { + setData("pending", sessionID, reconcile(result.pending)) + setData( + "input", + sessionID, + reconcile(result.pending.filter((item) => item.type !== "compaction").map((item) => item.id)), + ) + } + if (formStable) setData("form", sessionID, reconcile(result.forms)) + if (pendingStable && formStable) return } - if ((formRevision.get(sessionID) ?? 0) === formAt) setData("form", sessionID, reconcile(result.forms)) }, refreshPinned(hydrateTransient: (sessionID: string) => Promise) { return Promise.all( diff --git a/packages/app/src/context/server-sync.tsx b/packages/app/src/context/server-sync.tsx index 412ffb6d25..015187c20b 100644 --- a/packages/app/src/context/server-sync.tsx +++ b/packages/app/src/context/server-sync.tsx @@ -5,7 +5,7 @@ import { batch, createMemo, getOwner, onCleanup, untrack } from "solid-js" import { createStore, produce, reconcile } from "solid-js/store" import { useLanguage } from "@/context/language" import type { InitError } from "../pages/error" -import { ServerSDK } from "./server-sdk" +import { type ServerEvent, type ServerSDK } from "./server-sdk" import { bootstrapDirectory, bootstrapGlobal, @@ -69,6 +69,16 @@ type GlobalStore = { reload: undefined | "pending" | "complete" } +const SESSION_LIST_EVENTS = new Set([ + "session.created", + "session.updated", + "session.deleted", + "session.moved", + "session.forked", + "session.renamed", + "session.usage.updated", +]) + type McpListApi = { readonly list: (input?: McpListInput) => Promise } @@ -198,6 +208,7 @@ export function createServerSyncContextInner(serverSDK: ServerSDK) { const booting = new Map>() const sessionLoads = new Map>() const sessionMeta = new Map() + const sessionRevision = new Map() const session = createServerSession(serverSDK.api.session, serverSDK.api.message) const queryOptionsApi = makeQueryOptionsApi(serverSDK.scope, serverSDK.api) @@ -211,6 +222,7 @@ export function createServerSyncContextInner(serverSDK: ServerSDK) { return { pending, forms } }) } + const hydrateSession = (sessionID: string) => Promise.all([session.sync(sessionID), hydrateSessionState(sessionID)]) const [configQuery, providerQuery, pathQuery] = useQueries(() => ({ queries: [ @@ -384,6 +396,14 @@ export function createServerSyncContextInner(serverSDK: ServerSDK) { }, }) + async function loadCurrentSessions(directory: string, key: PathKey, limit: number) { + while (true) { + const revision = sessionRevision.get(key) ?? 0 + const result = await loadRootSessions({ api: serverSDK.api.session, directory, limit }) + if ((sessionRevision.get(key) ?? 0) === revision) return result + } + } + async function loadSessions(directory: string, options?: { limit?: number }) { const key = directoryKey(directory) const pending = sessionLoads.get(key) @@ -413,7 +433,7 @@ export function createServerSyncContextInner(serverSDK: ServerSDK) { .fetchQuery({ ...queryOptionsApi.sessions(key), queryFn: () => - loadRootSessions({ api: serverSDK.api.session, directory, limit }) + loadCurrentSessions(directory, key, limit) .then((x) => { const nonArchived = (x.data ?? []) .filter((s) => !!s?.id) @@ -517,12 +537,43 @@ export function createServerSyncContextInner(serverSDK: ServerSDK) { loadLsp() {}, }) } + const updateHomeSession = (info: Parameters[0]) => + homeSessions.apply({ + type: "session.updated", + properties: { sessionID: info.id, info }, + }) + const markSessionListChanged = (event: ServerEvent, directory: string, previousDirectory?: string) => { + if (SESSION_LIST_EVENTS.has(event.current?.type ?? event.type)) { + const key = directoryKey(directory) + sessionRevision.set(key, (sessionRevision.get(key) ?? 0) + 1) + } + if (!previousDirectory || previousDirectory === directory) return + const key = directoryKey(previousDirectory) + sessionRevision.set(key, (sessionRevision.get(key) ?? 0) + 1) + } + const toDirectoryEvent = (event: ServerEvent) => { + if (event.current?.type === "session.created") return + if ( + event.current?.type !== "session.renamed" && + event.current?.type !== "session.moved" && + event.current?.type !== "session.usage.updated" + ) + return event + const info = session.get(event.current.data.sessionID) + if (info) return { type: "session.updated", properties: { info } } + return event + } const unsub = serverSDK.event.listen((e) => { const directory = e.name const key = directoryKey(directory) const event = e.details const eventType: string = event.type + const previousDirectory = + event.current?.type === "session.moved" + ? session.get(event.current.data.sessionID)?.location.directory + : undefined + markSessionListChanged(event, directory, previousDirectory) if (event.current) session.applyV2(event.current) session.apply(event) if (event.current?.type === "session.created") @@ -550,12 +601,17 @@ export function createServerSyncContextInner(serverSDK: ServerSDK) { event.current?.type === "session.moved" || event.current?.type === "session.usage.updated" ) { - const info = session.get(event.current.data.sessionID) - if (info) - homeSessions.apply({ - type: "session.updated", - properties: { sessionID: info.id, info }, - }) + const sessionID = event.current.data.sessionID + const info = session.get(sessionID) + if (info) updateHomeSession(info) + if (!info) + void session + .resolve(sessionID) + .then(() => { + const current = session.get(sessionID) + if (current) updateHomeSession(current) + }) + .catch(() => undefined) } homeSessions.refresh(event.type) catalog.handleEvent({ type: eventType, directory }) @@ -613,27 +669,29 @@ export function createServerSyncContextInner(serverSDK: ServerSDK) { .then((commands) => setStore("command", commands)) .catch(() => {}) if (eventType === "project.directories.updated") void bootstrap.refetch() - applyDirectoryEvent({ - event, - directory, - store, - setStore, - push: (directory) => { - if (children.active(directory)) queue.push(directory) - }, - retainedLimit: sessionMeta.get(key)?.limit, - sessionContent: false, - permission: session.data.permission, - vcsCache: children.vcsCache.get(key), - loadLsp: () => { - if (!children.active(key)) return - void queryClient.fetchQuery(queryOptionsApi.lsp(key)) - }, - loadReferences: () => { - if (!children.active(key)) return - void queryClient.fetchQuery(queryOptionsApi.references(key)) - }, - }) + const projected = toDirectoryEvent(event) + if (projected) + applyDirectoryEvent({ + event: projected, + directory, + store, + setStore, + push: (directory) => { + if (children.active(directory)) queue.push(directory) + }, + retainedLimit: sessionMeta.get(key)?.limit, + sessionContent: false, + permission: session.data.permission, + vcsCache: children.vcsCache.get(key), + loadLsp: () => { + if (!children.active(key)) return + void queryClient.fetchQuery(queryOptionsApi.lsp(key)) + }, + loadReferences: () => { + if (!children.active(key)) return + void queryClient.fetchQuery(queryOptionsApi.references(key)) + }, + }) }) onCleanup(unsub) @@ -690,7 +748,7 @@ export function createServerSyncContextInner(serverSDK: ServerSDK) { // bootstrap, updateConfig: updateConfigMutation.mutateAsync, project: projectApi, - session, + session: Object.assign(session, { hydrate: hydrateSession }), homeSessions, mcp: { toggle: async (directory: string, name: string) => { diff --git a/packages/app/src/pages/directory-layout.tsx b/packages/app/src/pages/directory-layout.tsx index 7df092aceb..a8a02bd22d 100644 --- a/packages/app/src/pages/directory-layout.tsx +++ b/packages/app/src/pages/directory-layout.tsx @@ -44,8 +44,8 @@ export function DirectoryDataProvider( createResource( () => params.id, (id) => - sync() - .session.sync(id) + serverSync() + .session.hydrate(id) .catch(() => {}), ) diff --git a/packages/app/src/pages/home/home-sessions-controller.tsx b/packages/app/src/pages/home/home-sessions-controller.tsx index 37926f1cf9..fb5a5431c2 100644 --- a/packages/app/src/pages/home/home-sessions-controller.tsx +++ b/packages/app/src/pages/home/home-sessions-controller.tsx @@ -60,7 +60,7 @@ export function createHomeSessionsController(home: HomeController) { })) const sessionLoad = useQuery(() => ({ queryKey: homeSessions().indexKey, - enabled: !!home.server.focusedContext(), + enabled: home.server.focusedContext()?.sdk.connection.status() === "connected", queryFn: async ({ signal }) => { const ctx = home.server.focusedContext() if (!ctx) return { sessions: [], eventSequence: 0 } diff --git a/packages/app/src/pages/session.tsx b/packages/app/src/pages/session.tsx index ac6f44398f..b70fb6fb5b 100644 --- a/packages/app/src/pages/session.tsx +++ b/packages/app/src/pages/session.tsx @@ -651,11 +651,18 @@ export default function Page() { }) const vcsKey = createMemo( () => - ["session-vcs", sdk().directory, sync().data.vcs?.branch ?? "", sync().data.vcs?.default_branch ?? ""] as const, + [ + serverSDK().scope, + "session-vcs", + sdk().directory, + sync().data.vcs?.branch ?? "", + sync().data.vcs?.default_branch ?? "", + ] as const, ) const vcsQuery = createQuery(() => { const mode = vcsMode() - const enabled = wantsReview() && sync().project?.vcs === "git" + const enabled = + serverSDK().connection.status() === "connected" && wantsReview() && sync().project?.vcs === "git" return { queryKey: [...vcsKey(), mode] as const, @@ -667,16 +674,11 @@ export default function Page() { sdk() .api.vcs.diff({ location: { directory: sdk().directory }, mode: mode === "git" ? "working" : mode }) .then((result) => result.data) - .catch((error) => { - console.debug("[session-review] failed to load vcs diff", { mode, error }) - return [] - }) : skipToken, } }) const refreshVcs = debounce(() => { void queryClient.invalidateQueries({ queryKey: vcsKey() }) - void queryClient.invalidateQueries({ queryKey: [serverSDK().scope, ...vcsKey()] }) }, 100) onCleanup( sdk().event.listen((event) => { @@ -726,7 +728,7 @@ export default function Page() { const request = (scope: string, context?: number) => queryClient .fetchQuery({ - queryKey: [serverSDK().scope, ...vcsKey(), mode, "directory", scope, context, version] as const, + queryKey: [...vcsKey(), mode, "directory", scope, context, version] as const, staleTime: Number.POSITIVE_INFINITY, retry: 2, queryFn: () => diff --git a/packages/app/src/pages/session/composer/session-question-dock.tsx b/packages/app/src/pages/session/composer/session-question-dock.tsx index 6a2f4a1bf8..c321662749 100644 --- a/packages/app/src/pages/session/composer/session-question-dock.tsx +++ b/packages/app/src/pages/session/composer/session-question-dock.tsx @@ -263,18 +263,18 @@ export const SessionQuestionDock: Component<{ request: FormInfo; onSubmit: () => const sending = createMemo(() => replyMutation.isPending || rejectMutation.isPending) - const reply = async (answer: FormAnswer) => { + const reply = (answer: FormAnswer) => { if (sending()) return - await replyMutation.mutateAsync(answer) + replyMutation.mutate(answer) } - const reject = async () => { + const reject = () => { if (sending()) return - await rejectMutation.mutateAsync() + rejectMutation.mutate() } const submit = () => - void reply( + reply( Object.fromEntries( questions().flatMap((question, index) => { const answers = store.answers[index] ?? [] @@ -348,7 +348,7 @@ export const SessionQuestionDock: Component<{ request: FormInfo; onSubmit: () => if (event.key === "Escape") { event.preventDefault() - void reject() + reject() return } diff --git a/packages/app/src/pages/session/v2/session-file-browser-tab.tsx b/packages/app/src/pages/session/v2/session-file-browser-tab.tsx index 639429e80b..b557028326 100644 --- a/packages/app/src/pages/session/v2/session-file-browser-tab.tsx +++ b/packages/app/src/pages/session/v2/session-file-browser-tab.tsx @@ -8,6 +8,7 @@ import { useFile } from "@/context/file" import { useLanguage } from "@/context/language" import { useLayout } from "@/context/layout" import { useSDK } from "@/context/sdk" +import { useServerSDK } from "@/context/server-sdk" import { displayName } from "@/pages/layout/helpers" import { useSessionLayout } from "@/pages/session/session-layout" import { SessionFileView } from "@/pages/session/file-tabs" @@ -38,6 +39,7 @@ export function SessionFileBrowserTab(props: { const language = useLanguage() const layout = useLayout() const sdk = useSDK() + const serverSDK = useServerSDK() const { workspaceKey } = useSessionLayout() const resultsID = `session-file-browser-results-${createUniqueId()}` const [filter, setFilter] = createSignal("") @@ -47,8 +49,8 @@ export function SessionFileBrowserTab(props: { const search = createQuery(() => { const value = query() return { - queryKey: ["session-open-file", workspaceKey(), value] as const, - enabled: value.length > 0, + queryKey: [serverSDK().scope, "session-open-file", workspaceKey(), value] as const, + enabled: serverSDK().connection.status() === "connected" && value.length > 0, queryFn: ({ signal }) => file.searchFiles(value, { limit: 200, signal }), } })