From 54dbbddc0c4e3fc41197ff92271ea0bbc108e176 Mon Sep 17 00:00:00 2001 From: Luke Parker <10430890+Hona@users.noreply.github.com> Date: Thu, 5 Nov 2026 00:26:34 -0600 Subject: [PATCH] fix(app): drive plugin link checks, located writes and side layout through state machines (#61967) --- packages/app/src/runtime/plugin/host-apis.tsx | 265 +++++++++++------- .../plugin/host-layout-machine.test.ts | 200 +++++++++++++ .../src/runtime/plugin/host-layout-machine.ts | 250 +++++++++++++++++ .../runtime/plugin/link-check-machine.test.ts | 115 ++++++++ .../src/runtime/plugin/link-check-machine.ts | 142 ++++++++++ packages/app/src/runtime/plugin/link-check.ts | 83 ++++-- .../runtime/plugin/located-machine.test.ts | 124 ++++++++ .../app/src/runtime/plugin/located-machine.ts | 125 +++++++++ packages/app/src/runtime/plugin/located.ts | 80 +++++- .../test-browser/plugin-link-check.test.ts | 17 ++ .../test-browser/plugin-primitives.test.ts | 24 +- 11 files changed, 1292 insertions(+), 133 deletions(-) create mode 100644 packages/app/src/runtime/plugin/host-layout-machine.test.ts create mode 100644 packages/app/src/runtime/plugin/host-layout-machine.ts create mode 100644 packages/app/src/runtime/plugin/link-check-machine.test.ts create mode 100644 packages/app/src/runtime/plugin/link-check-machine.ts create mode 100644 packages/app/src/runtime/plugin/located-machine.test.ts create mode 100644 packages/app/src/runtime/plugin/located-machine.ts diff --git a/packages/app/src/runtime/plugin/host-apis.tsx b/packages/app/src/runtime/plugin/host-apis.tsx index 5139e45f124c..d74740f03afc 100644 --- a/packages/app/src/runtime/plugin/host-apis.tsx +++ b/packages/app/src/runtime/plugin/host-apis.tsx @@ -44,7 +44,7 @@ import { ServerConnection, useServers } from "@/runtime/server/registry" import { isServerIncompatible } from "@/runtime/server/health-machine" import { useDirectoryPicker } from "@/workspaces/selection/picker" import { SessionRouteKey, SessionStateKey, type ServerScope } from "@/runtime/server/scope" -import { findSessionTab, tabKey, useTabs } from "@/shell/tabs/tabs" +import { findSessionTab, tabKey, useTabs, type Tab } from "@/shell/tabs/tabs" import { openNotificationSession } from "@/shell/notifications/notification" import { playSoundById } from "@/shell/notifications/sound" import { showToast } from "@/shell/notifications/toast" @@ -67,6 +67,19 @@ import { import { closedEntry, createClosedTabs } from "./closed-tabs" import { panelsMoving } from "@/shell/panel-motion" import { createEmbeds } from "./embeds" +import { + createMobileViewState, + createProjectRequestState, + createSideRegionState, + selectMobileView, + selectWaitingProjectServer, + transitionMobileView, + transitionProjectRequest, + transitionSideRegion, + type MobileViewEvent, + type ProjectRequestEvent, + type SideRegionEvent, +} from "./host-layout-machine" import { usePluginHost, type HostApiFactories } from "./host" import { createLocatedWrites } from "./located" import { createSessionPanelMenu } from "./panel-menu" @@ -271,12 +284,13 @@ export function createHostApis() { const run = (connected: Attached) => removePersisted(target(plugin, key, scope, options?.from, builtin, connected), platform) - // A session's store is found through its location: the removal waits for it, as layout writes do. - write((connected) => - "session" in scope && !scope.session.location - ? located.hold(scope.session, () => write(run)) - : run(connected), - ) + // A session's store is found through its location: the removal waits for it, behind the session's earlier + // held removals, as layout writes do. + write((connected) => { + if (!("session" in scope) || (scope.session.location && !located.pending(scope.session.key))) + return run(connected) + located.hold(scope.session, () => write(run)) + }) }, }), system: () => ({ @@ -696,20 +710,34 @@ export function createPluginAttachment(apis: HostApis) { true, ) - // A token per session whose side region is open, new each time the region opens. - const sideVisits = createMemo>( - (previous) => - new Map( - sessions().flatMap((session) => - sideOpened(session) ? [[session.key, previous.get(session.key) ?? {}] as const] : [], - ), - ), - new Map(), - ) + // The keys a session tab's sessions go by: the session it shows and the one it routes. + const tabSessions = (tab: Tab) => + tab.type === "session" + ? [`${tab.server}\n${tab.sessionId}`, `${tab.server}\n${tab.routeSessionId ?? tab.sessionId}`] + : [] - // The panel whose toggle opened a side region, by the region's token. However the region closes, it reopens with - // a new token, so a region reopened any other way belongs to the user. - const openedFor = new WeakMap() + // Who each open side region belongs to: the panel whose `Layout.toggle` opened it, or the user. Read straight from + // the shell tabs, so a session missing from `sessions()` (e.g. while its server signs in again) keeps its owner. + const sideRegion = { state: createSideRegionState() } + + const dispatchSide = (event: SideRegionEvent) => { + const next = transitionSideRegion(sideRegion.state, event) + + sideRegion.state = next.state + next.effects.forEach((effect) => + tabs.setRegion( + tabs.store.find((tab) => tabSessions(tab).includes(effect.session)), + "side", + false, + ), + ) + } + + createComputed(() => { + const opened = new Set(tabs.store.flatMap((tab) => (tabs.region(tab, "side") ? tabSessions(tab) : []))) + + untrack(() => dispatchSide({ type: "sync", opened })) + }) // Keys are `${plugin}:${tab id}`. const provider = (key: string) => { @@ -724,24 +752,28 @@ export function createPluginAttachment(apis: HostApis) { return view?.key === session.key ? view : undefined } - // Counts routing visits: each change of the routed session, including to none (e.g. Home), starts the next one. - const visit = createMemo(on(routed, (_key, _previous, count: number = 0) => count + 1)) + // The narrow-screen view belongs to the routed, mounted session for one routing visit, and reads as the conversation + // once another visit starts. A view selected for a session that is not routed (e.g. a file link that opens Files on + // another session) belongs to the next visit only when the navigation that follows routes that session. + const [mobileState, setMobileState] = createSignal(createMobileViewState()) + let mobileTimer: ReturnType | undefined - // The narrow-screen view belongs to the routed, mounted session for one visit, and reads as the conversation once - // another visit starts. A view selected for a session that is not routed (e.g. a file link that opens Files on - // another session) belongs to the next visit, which the navigation that follows starts. - const [mobile, setMobile] = createStore<{ session: string | undefined; view: string; visit: number }>({ - session: undefined, - view: "session", - visit: 0, - }) + const dispatchMobile = (event: MobileViewEvent) => { + const next = transitionMobileView(untrack(mobileState), event) - const mobileView = createMemo(() => - mobile.session === current()?.key && mobile.visit === visit() ? mobile.view : "session", - ) + setMobileState(next.state) + next.effects.forEach((effect) => { + if (effect.type === "cancel.expire") return clearTimeout(mobileTimer) + mobileTimer = setTimeout(() => dispatchMobile({ type: "expire", attempt: effect.attempt }), effect.timeoutMs) + }) + } + + createComputed(on(routed, (key) => dispatchMobile({ type: "route.changed", routed: key }), { defer: true })) + + const mobileView = createMemo(() => selectMobileView(mobileState(), current()?.key)) const selectMobile = (session: SessionRef, view: string) => - setMobile({ session: session.key, view, visit: session.key === routed() ? visit() : visit() + 1 }) + dispatchMobile({ type: "select", session: session.key, view, routed: untrack(routed) }) // The side tabs a mounted session lists right now, plus `adding` as if it were stored; unmounted sessions have none. const listed = (session: SessionRef, value: string, adding?: string) => { @@ -773,9 +805,17 @@ export function createPluginAttachment(apis: HostApis) { ) } - // Writes made before a session's location is known wait for it rather than being dropped. + // Writes made before a session's location is known wait for it rather than being dropped. A session's writes run in + // call order: one made while earlier ones wait, or by one of them, joins the end. const located = createLocatedWrites() + const withLocated = (session: SessionRef, run: (value: string) => void) => + located.hold(session, () => { + const value = stateKey(session) + + if (value) run(value) + }) + // Kept per session beside its strip, so the new tab view still lists them after a reload. const closed = createClosedTabs({ list(session) { @@ -790,12 +830,32 @@ export function createPluginAttachment(apis: HostApis) { }, }) - const open = (key: string, session: SessionRef, options?: OpenOptions) => { + // Opens a tab in the session's located strip `value`, and records whether that opened the side region for it. + const openAt = ( + key: string, + session: SessionRef, + value: string, + options: OpenOptions | undefined, + via: "open" | "toggle", + ) => { + const wasOpen = sideOpened(session) + + batch(() => { + place(key, session, value, options) + dispatchSide({ + type: "panel.opened", + session: session.key, + key, + via, + placement: options?.tab ?? "open", + wasOpen, + nowOpen: sideOpened(session), + }) + }) + } + + const place = (key: string, session: SessionRef, value: string, options: OpenOptions | undefined) => { const item = provider(key) - - const value = stateKey(session) - - if (!value) return located.hold(session, () => open(key, session, options)) const placement = options?.tab ?? "open" // An append adds the tab quietly: no selection, no region change, no preview replacement. @@ -830,13 +890,11 @@ export function createPluginAttachment(apis: HostApis) { }) } - const close = (key: string, session: SessionRef) => { + const open = (key: string, session: SessionRef, options?: OpenOptions) => + withLocated(session, (value) => openAt(key, session, value, options, "open")) + + const closeAt = (key: string, session: SessionRef, value: string) => { const item = provider(key) - - const value = stateKey(session) - - if (!value) return located.hold(session, () => close(key, session)) - const tab = listed(session, value).find((entry) => entry.key === key)?.tab const all = layout.panel.state(value).all const view = mountedSession(session) @@ -860,6 +918,8 @@ export function createPluginAttachment(apis: HostApis) { if (view && tab && currentScreen) item?.value.close?.({ tab, session: view, screen: currentScreen }) } + const close = (key: string, session: SessionRef) => withLocated(session, (value) => closeAt(key, session, value)) + // The routed session's side region, which knows the fallback selection the stored state lacks. const [region, setRegion] = createSignal() // The session screen's inner sidebar preference, which its side panels share. @@ -885,35 +945,40 @@ export function createPluginAttachment(apis: HostApis) { return paneOpened(session, pane) ? "visible" : "active" } - const hide = (key: string, session: SessionRef) => { - const value = stateKey(session) - - if (!value) return located.hold(session, () => hide(key, session)) - - if (state(key, session) !== "visible") return - const pane = mountedSession(session) ? region()?.paneOf(key) : sessionTabPane(layout.panel.state(value), key) - tabs.setRegion(shellTab(session), pane === "bottom" ? "bottom" : "side", false) - } - - // Open-project requests wait until their server is listed (e.g. an SSH server that just connected). - const picker = useDirectoryPicker() - const [projects, setProjects] = createSignal([]) - createEffect(() => { - const pending = projects() - - const ready = pending.flatMap((request) => { - const server = servers.list.find((conn) => ServerConnection.key(conn) === request.server) - - return server ? [{ request, server }] : [] + const hide = (key: string, session: SessionRef) => + withLocated(session, (value) => { + if (state(key, session) !== "visible") return + const pane = mountedSession(session) ? region()?.paneOf(key) : sessionTabPane(layout.panel.state(value), key) + tabs.setRegion(shellTab(session), pane === "bottom" ? "bottom" : "side", false) }) - if (ready.length === 0) return - setProjects(pending.filter((request) => !ready.some((item) => item.request === request))) - untrack(() => - ready.forEach(({ request, server }) => + // An open-project request waits until its server is listed (e.g. an SSH server that just connected), for at most + // `PROJECT_REQUEST_TTL_MS`; a newer request replaces it. + const picker = useDirectoryPicker() + const [projectState, setProjectState] = createSignal(createProjectRequestState()) + let projectTimer: ReturnType | undefined + + const listedServer = (id: string) => servers.list.find((conn) => ServerConnection.key(conn) === id) + + const dispatchProject = (event: ProjectRequestEvent) => { + const next = transitionProjectRequest(untrack(projectState), event) + + setProjectState(next.state) + next.effects.forEach((effect) => { + if (effect.type === "cancel.expire") return clearTimeout(projectTimer) + + if (effect.type === "schedule.expire") + return void (projectTimer = setTimeout( + () => dispatchProject({ type: "expire", attempt: effect.attempt }), + effect.timeoutMs, + )) + const server = untrack(() => listedServer(effect.server)) + + if (!server) return + untrack(() => picker({ server, - title: request.title, + title: effect.title, onSelect: (value) => { const directory = Array.isArray(value) ? value[0] : value @@ -923,42 +988,39 @@ export function createPluginAttachment(apis: HostApis) { void tabs.newDraft({ server: key, directory }) }, }), - ), - ) + ) + }) + } + + createEffect(() => { + const server = selectWaitingProjectServer(projectState()) + + if (server && listedServer(server)) untrack(() => dispatchProject({ type: "server.listed", server })) }) - const toggle = (key: string, session: SessionRef) => { - const value = stateKey(session) + if (owner) + onCleanup(() => { + dispatchMobile({ type: "disposed" }) + dispatchProject({ type: "disposed" }) + }) - if (!value) return located.hold(session, () => toggle(key, session)) - const region = sideVisits().get(session.key) + const toggle = (key: string, session: SessionRef) => + withLocated(session, (value) => { + if (state(key, session) !== "visible") return openAt(key, session, value, undefined, "toggle") - if (state(key, session) === "visible") { batch(() => { - close(key, session) - - // Closing the last panel the region was opened for also closes the region. - if (region && openedFor.get(region) === key && layout.panel.state(value).all.length === 0) - tabs.setRegion(shellTab(session), "side", false) + closeAt(key, session, value) + dispatchSide({ + type: "panel.closed", + session: session.key, + key, + empty: layout.panel.state(value).all.length === 0, + }) }) + }) - return - } - - // A panel opened into an open region makes the region the user's. - if (region) openedFor.delete(region) - open(key, session) - const opened = sideVisits().get(session.key) - - if (!region && opened) openedFor.set(opened, key) - } - - const setScroll = (session: SessionRef, key: string, next: { readonly x: number; readonly y: number }) => { - const value = stateKey(session) - - if (!value) return located.hold(session, () => setScroll(session, key, next)) - layout.panel.setScroll(value, key, next) - } + const setScroll = (session: SessionRef, key: string, next: { readonly x: number; readonly y: number }) => + withLocated(session, (value) => layout.panel.setScroll(value, key, next)) // The tab the route shows: a session's, a draft's, or none (Home, Settings). const routedTab = () => { @@ -1083,7 +1145,8 @@ export function createPluginAttachment(apis: HostApis) { screen, scope, scoped: layout.sessionState.track, - project: (server, title) => setProjects((pending) => [...pending, { server, title }]), + project: (server, title) => + dispatchProject({ type: "request", server, title, listed: !!untrack(() => listedServer(server)) }), draft: (input) => { const server = servers.list.find((conn) => ServerConnection.key(conn) === input.server) diff --git a/packages/app/src/runtime/plugin/host-layout-machine.test.ts b/packages/app/src/runtime/plugin/host-layout-machine.test.ts new file mode 100644 index 000000000000..8073a1a4ae0f --- /dev/null +++ b/packages/app/src/runtime/plugin/host-layout-machine.test.ts @@ -0,0 +1,200 @@ +import { expect, test } from "bun:test" +import { + createMobileViewState, + createProjectRequestState, + createSideRegionState, + MOBILE_VIEW_PENDING_TTL_MS, + PROJECT_REQUEST_TTL_MS, + selectMobileView, + selectSideRegionSession, + transitionMobileView, + transitionProjectRequest, + transitionSideRegion, + type MobileViewEvent, + type MobileViewState, + type ProjectRequestEvent, + type ProjectRequestState, + type SideRegionEvent, + type SideRegionState, +} from "./host-layout-machine" + +const s = "server\nses_a" + +function fold(transition: (state: S, event: E) => { state: S; effects: readonly X[] }, initial: S) { + const machine = { state: initial } + const effects: X[] = [] + + const dispatch = (event: E) => { + const next = transition(machine.state, event) + + machine.state = next.state + effects.push(...next.effects) + + return next + } + + return { machine, effects, dispatch } +} + +const side = () => fold(transitionSideRegion, createSideRegionState()) + +const toggled = { type: "panel.opened", session: s, key: "context:main", via: "toggle", placement: "open" } as const + +// Before, `Layout.open` never cleared the toggle's ownership, so toggling the first panel off shut the region over the +// panel opened after it. +test("a panel opened into a region a toggle opened makes the region the user's", () => { + const run = side() + + run.dispatch({ ...toggled, wasOpen: false, nowOpen: true }) + expect(selectSideRegionSession(run.machine.state, s)).toEqual({ status: "open-for-panel", key: "context:main" }) + + run.dispatch({ + type: "panel.opened", + session: s, + key: "review:review", + via: "open", + placement: "open", + wasOpen: true, + nowOpen: true, + }) + run.dispatch({ type: "panel.closed", session: s, key: "context:main", empty: true }) + + expect(selectSideRegionSession(run.machine.state, s)).toEqual({ status: "open-user" }) + expect(run.effects).toEqual([]) +}) + +// Before, ownership hung off a token minted from `sessions()`, so a session that dropped out of it while its server +// signed in again came back with a new token and lost its owner. +test("a sync that still lists the open region keeps its owner, and only a close forgets it", () => { + const run = side() + + run.dispatch({ ...toggled, wasOpen: false, nowOpen: true }) + const owned = run.machine.state + + expect(run.dispatch({ type: "sync", opened: new Set([s]) }).state).toBe(owned) + run.dispatch({ type: "panel.closed", session: s, key: "context:main", empty: true }) + expect(run.effects).toEqual([{ type: "close.side", session: s }]) + + run.dispatch({ ...toggled, wasOpen: false, nowOpen: true }) + run.dispatch({ type: "sync", opened: new Set() }) + run.dispatch({ type: "sync", opened: new Set([s]) }) + expect(selectSideRegionSession(run.machine.state, s)).toEqual({ status: "open-user" }) +}) + +test("a quiet append, or reopening the owner panel, keeps the region the panel's", () => { + const run = side() + + run.dispatch({ ...toggled, wasOpen: false, nowOpen: true }) + const owned = run.machine.state + + run.dispatch({ + type: "panel.opened", + session: s, + key: "review:review", + via: "open", + placement: "append", + wasOpen: true, + nowOpen: true, + }) + run.dispatch({ + type: "panel.opened", + session: s, + key: "context:main", + via: "open", + placement: "open", + wasOpen: true, + nowOpen: true, + }) + expect(run.machine.state).toBe(owned) + + run.dispatch({ ...toggled, wasOpen: true, nowOpen: true }) + expect(selectSideRegionSession(run.machine.state, s)).toEqual({ status: "open-user" }) +}) + +const mobile = () => fold(transitionMobileView, createMobileViewState()) + +// Before, a view selected for a session that was not routed waited for the next visit forever, so tapping that session +// minutes later opened Files instead of the conversation. +test("a pending narrow-screen view expires, and a visit to another session drops it", () => { + const expired = mobile() + + expired.dispatch({ type: "select", session: s, view: "file:files", routed: undefined }) + expired.dispatch({ type: "expire", attempt: 1 }) + expired.dispatch({ type: "route.changed", routed: s }) + expect(selectMobileView(expired.machine.state, s)).toBe("session") + expect(expired.effects).toEqual([{ type: "schedule.expire", attempt: 1, timeoutMs: MOBILE_VIEW_PENDING_TTL_MS }]) + + const elsewhere = mobile() + + elsewhere.dispatch({ type: "select", session: s, view: "file:files", routed: undefined }) + elsewhere.dispatch({ type: "route.changed", routed: "server\nses_b" }) + elsewhere.dispatch({ type: "route.changed", routed: s }) + expect(selectMobileView(elsewhere.machine.state, s)).toBe("session") +}) + +test("a pending view becomes the view of the visit that routes its session, until the next visit", () => { + const run = mobile() + + run.dispatch({ type: "select", session: s, view: "file:files", routed: undefined }) + run.dispatch({ type: "route.changed", routed: s }) + const active = run.machine.state + + expect(selectMobileView(active, s)).toBe("file:files") + expect(run.dispatch({ type: "expire", attempt: 1 }).state).toBe(active) + run.dispatch({ type: "route.changed", routed: undefined }) + expect(selectMobileView(run.machine.state, s)).toBe("session") + expect(run.effects).toEqual([ + { type: "schedule.expire", attempt: 1, timeoutMs: MOBILE_VIEW_PENDING_TTL_MS }, + { type: "cancel.expire" }, + ]) +}) + +test("disposing cancels a pending view's timer", () => { + const run = mobile() + + run.dispatch({ type: "select", session: s, view: "file:files", routed: undefined }) + run.dispatch({ type: "disposed" }) + const disposed = run.machine.state + + expect(run.dispatch({ type: "route.changed", routed: s }).state).toBe(disposed) + expect(run.effects.at(-1)).toEqual({ type: "cancel.expire" }) +}) + +const project = () => + fold(transitionProjectRequest, createProjectRequestState()) + +// Before, every request waited for its server forever, so clicking twice while an SSH server connected opened two +// pickers, and one for a server that never connected popped up whenever it later did. +test("a newer open-project request replaces one that waits, and an expired one opens nothing", () => { + const run = project() + + run.dispatch({ type: "request", server: "ssh:a", title: "Open", listed: false }) + run.dispatch({ type: "request", server: "ssh:a", title: "Open", listed: false }) + run.dispatch({ type: "server.listed", server: "ssh:a" }) + run.dispatch({ type: "server.listed", server: "ssh:a" }) + + expect(run.effects).toEqual([ + { type: "schedule.expire", attempt: 1, timeoutMs: PROJECT_REQUEST_TTL_MS }, + { type: "cancel.expire" }, + { type: "schedule.expire", attempt: 2, timeoutMs: PROJECT_REQUEST_TTL_MS }, + { type: "cancel.expire" }, + { type: "open.picker", server: "ssh:a", title: "Open" }, + ]) + + const stale = project() + + stale.dispatch({ type: "request", server: "ssh:b", title: "Open", listed: false }) + stale.dispatch({ type: "expire", attempt: 1 }) + stale.dispatch({ type: "server.listed", server: "ssh:b" }) + expect(stale.effects).toEqual([{ type: "schedule.expire", attempt: 1, timeoutMs: PROJECT_REQUEST_TTL_MS }]) +}) + +test("a request for a listed server opens the picker at once", () => { + const run = project() + const idle = run.machine.state + + expect(run.dispatch({ type: "request", server: "local", title: "Open", listed: true }).state).toBe(idle) + run.dispatch({ type: "disposed" }) + run.dispatch({ type: "request", server: "local", title: "Open", listed: true }) + expect(run.effects).toEqual([{ type: "open.picker", server: "local", title: "Open" }]) +}) diff --git a/packages/app/src/runtime/plugin/host-layout-machine.ts b/packages/app/src/runtime/plugin/host-layout-machine.ts new file mode 100644 index 000000000000..1eb19a7462da --- /dev/null +++ b/packages/app/src/runtime/plugin/host-layout-machine.ts @@ -0,0 +1,250 @@ +import type { OpenOptions } from "@opencode/plugin/gui" + +/** A narrow-screen view selected for a session that is not routed waits this long for the navigation that routes it. */ +export const MOBILE_VIEW_PENDING_TTL_MS = 5_000 + +/** An open-project request waits this long for its server to be listed (e.g. an SSH server connecting). */ +export const PROJECT_REQUEST_TTL_MS = 30_000 + +/** Who an open side region belongs to: the panel whose toggle opened it, or the user. */ +export type OpenSideRegion = + | { readonly status: "open-for-panel"; readonly key: string } + | { readonly status: "open-user" } + +export type SideRegionSessionState = { readonly status: "closed" } | OpenSideRegion + +/** Each session's open side region, by session key; a session that is absent has its side region closed. */ +export type SideRegionState = ReadonlyMap + +export type SideRegionEvent = + /** The sessions whose side region is open now, read from the shell tabs. */ + | { readonly type: "sync"; readonly opened: ReadonlySet } + | { + readonly type: "panel.opened" + readonly session: string + readonly key: string + readonly via: "open" | "toggle" + readonly placement: NonNullable + readonly wasOpen: boolean + readonly nowOpen: boolean + } + /** `Layout.toggle` closed a visible panel; `empty` when the session's strip has no stored tab left. */ + | { readonly type: "panel.closed"; readonly session: string; readonly key: string; readonly empty: boolean } + +export type SideRegionEffect = { readonly type: "close.side"; readonly session: string } + +export type SideRegionTransition = { readonly state: SideRegionState; readonly effects: readonly SideRegionEffect[] } + +export function createSideRegionState(): SideRegionState { + return new Map() +} + +export function transitionSideRegion(state: SideRegionState, event: SideRegionEvent): SideRegionTransition { + if (event.type === "sync") { + if (state.size === event.opened.size && Array.from(event.opened).every((key) => state.has(key))) + return { state, effects: [] } + + // A region that closed, however it closed, forgets its panel; one opened any other way is the user's. + return { + state: new Map(Array.from(event.opened, (key) => [key, state.get(key) ?? { status: "open-user" }] as const)), + effects: [], + } + } + + const current = state.get(event.session) + + if (event.type === "panel.closed") { + if (!event.empty || current?.status !== "open-for-panel" || current.key !== event.key) return { state, effects: [] } + + // Closing the last panel the region was opened for also closes the region. + return { state: without(state, event.session), effects: [{ type: "close.side", session: event.session }] } + } + + if (!event.nowOpen) return { state: current ? without(state, event.session) : state, effects: [] } + + if (!event.wasOpen) + return { + state: new Map(state).set( + event.session, + event.via === "toggle" ? { status: "open-for-panel", key: event.key } : { status: "open-user" }, + ), + effects: [], + } + + // A quiet append, or opening the panel the region belongs to, keeps it; any other panel opened into the open region + // makes it the user's. + if (current?.status === "open-user") return { state, effects: [] } + + if ( + current?.status === "open-for-panel" && + event.via === "open" && + (event.placement === "append" || current.key === event.key) + ) + return { state, effects: [] } + + return { state: new Map(state).set(event.session, { status: "open-user" }), effects: [] } +} + +export function selectSideRegionSession(state: SideRegionState, session: string): SideRegionSessionState { + return state.get(session) ?? { status: "closed" } +} + +function without(state: SideRegionState, session: string) { + const next = new Map(state) + next.delete(session) + + return next +} + +/** + * The narrow-screen view for one routing visit. `pending` is a view selected for a session that is not routed (e.g. a + * file link that opens Files on another session): it belongs to the next visit only if that visit routes its session + * within `MOBILE_VIEW_PENDING_TTL_MS`. `attempt` names the latest pending view, whose expiry alone applies. + */ +export type MobileViewState = + | { readonly status: "conversation"; readonly attempt: number } + | { readonly status: "active"; readonly attempt: number; readonly session: string; readonly view: string } + | { readonly status: "pending"; readonly attempt: number; readonly session: string; readonly view: string } + | { readonly status: "disposed" } + +export type MobileViewEvent = + | { readonly type: "select"; readonly session: string; readonly view: string; readonly routed: string | undefined } + | { readonly type: "route.changed"; readonly routed: string | undefined } + | { readonly type: "expire"; readonly attempt: number } + | { readonly type: "disposed" } + +export type MobileViewEffect = + | { readonly type: "schedule.expire"; readonly attempt: number; readonly timeoutMs: number } + | { readonly type: "cancel.expire" } + +export type MobileViewTransition = { readonly state: MobileViewState; readonly effects: readonly MobileViewEffect[] } + +export function createMobileViewState(): MobileViewState { + return { status: "conversation", attempt: 0 } +} + +export function transitionMobileView(state: MobileViewState, event: MobileViewEvent): MobileViewTransition { + if (state.status === "disposed") return { state, effects: [] } + const cancel: MobileViewEffect[] = state.status === "pending" ? [{ type: "cancel.expire" }] : [] + + if (event.type === "disposed") return { state: { status: "disposed" }, effects: cancel } + + if (event.type === "select") { + if (event.view === "session") + return { + state: state.status === "conversation" ? state : { status: "conversation", attempt: state.attempt }, + effects: cancel, + } + + if (event.session === event.routed) + return { + state: { status: "active", attempt: state.attempt, session: event.session, view: event.view }, + effects: cancel, + } + const attempt = state.attempt + 1 + + return { + state: { status: "pending", attempt, session: event.session, view: event.view }, + effects: [...cancel, { type: "schedule.expire", attempt, timeoutMs: MOBILE_VIEW_PENDING_TTL_MS }], + } + } + + if (event.type === "expire") { + if (state.status !== "pending" || event.attempt !== state.attempt) return { state, effects: [] } + + return { state: { status: "conversation", attempt: state.attempt }, effects: [] } + } + + if (state.status === "conversation") return { state, effects: [] } + + if (state.status === "active" && event.routed === state.session) return { state, effects: [] } + + // The navigation that follows a pending view starts its visit; any other visit reads as the conversation. + if (state.status === "pending" && event.routed === state.session) + return { + state: { status: "active", attempt: state.attempt, session: state.session, view: state.view }, + effects: cancel, + } + + return { state: { status: "conversation", attempt: state.attempt }, effects: cancel } +} + +/** The view the mounted session shows on a narrow screen; `"session"` is the conversation. */ +export function selectMobileView(state: MobileViewState, mounted: string | undefined) { + return state.status === "active" && state.session === mounted ? state.view : "session" +} + +/** One open-project request; a newer request replaces one that still waits for its server. */ +export type ProjectRequestState = + | { readonly status: "idle"; readonly attempt: number } + | { + readonly status: "waiting" + readonly attempt: number + readonly server: string + readonly title: string + } + | { readonly status: "disposed" } + +export type ProjectRequestEvent = + | { readonly type: "request"; readonly server: string; readonly title: string; readonly listed: boolean } + | { readonly type: "server.listed"; readonly server: string } + | { readonly type: "expire"; readonly attempt: number } + | { readonly type: "disposed" } + +export type ProjectRequestEffect = + | { readonly type: "open.picker"; readonly server: string; readonly title: string } + | { readonly type: "schedule.expire"; readonly attempt: number; readonly timeoutMs: number } + | { readonly type: "cancel.expire" } + +export type ProjectRequestTransition = { + readonly state: ProjectRequestState + readonly effects: readonly ProjectRequestEffect[] +} + +export function createProjectRequestState(): ProjectRequestState { + return { status: "idle", attempt: 0 } +} + +export function transitionProjectRequest( + state: ProjectRequestState, + event: ProjectRequestEvent, +): ProjectRequestTransition { + if (state.status === "disposed") return { state, effects: [] } + const cancel: ProjectRequestEffect[] = state.status === "waiting" ? [{ type: "cancel.expire" }] : [] + + if (event.type === "disposed") return { state: { status: "disposed" }, effects: cancel } + + if (event.type === "request") { + if (event.listed) + return { + state: state.status === "idle" ? state : { status: "idle", attempt: state.attempt }, + effects: [...cancel, { type: "open.picker", server: event.server, title: event.title }], + } + const attempt = state.attempt + 1 + + return { + state: { status: "waiting", attempt, server: event.server, title: event.title }, + effects: [...cancel, { type: "schedule.expire", attempt, timeoutMs: PROJECT_REQUEST_TTL_MS }], + } + } + + if (state.status !== "waiting") return { state, effects: [] } + + if (event.type === "server.listed") { + if (event.server !== state.server) return { state, effects: [] } + + return { + state: { status: "idle", attempt: state.attempt }, + effects: [...cancel, { type: "open.picker", server: state.server, title: state.title }], + } + } + + if (event.attempt !== state.attempt) return { state, effects: [] } + + return { state: { status: "idle", attempt: state.attempt }, effects: [] } +} + +/** The server a waiting request needs listed. */ +export function selectWaitingProjectServer(state: ProjectRequestState) { + return state.status === "waiting" ? state.server : undefined +} diff --git a/packages/app/src/runtime/plugin/link-check-machine.test.ts b/packages/app/src/runtime/plugin/link-check-machine.test.ts new file mode 100644 index 000000000000..8ecbceb53946 --- /dev/null +++ b/packages/app/src/runtime/plugin/link-check-machine.test.ts @@ -0,0 +1,115 @@ +import { expect, test } from "bun:test" +import { + createLinkCheckState, + transitionLinkCheck, + type LinkCheckEffect, + type LinkCheckEvent, +} from "./link-check-machine" + +/** Drives the machine as `createLinkCheck` does, recording every effect it asks for. */ +function harness(active = true) { + const machine = { state: createLinkCheckState(active) } + const effects: LinkCheckEffect[] = [] + + const dispatch = (event: LinkCheckEvent) => { + const next = transitionLinkCheck(machine.state, event) + + machine.state = next.state + effects.push(...next.effects) + + return next + } + + return { machine, effects, dispatch } +} + +// Before, a check waiting on a hidden timeline, or one whose negative answer came back after the timeline was +// disposed, never settled: its markdown link stayed pending forever. +test("disposing settles every waiting and running check false, and ignores a late answer", () => { + const run = harness(false) + const waiting = run.dispatch({ type: "defer", href: "a.ts" }).id ?? 0 + const running = run.dispatch({ type: "start", href: "b.ts" }).id ?? 0 + + run.dispatch({ type: "disposed" }) + const disposed = run.machine.state + const late = run.dispatch({ type: "settled", id: running, found: false }) + + expect(run.effects).toEqual([ + { type: "resolve", id: waiting, value: false }, + { type: "resolve", id: running, value: false }, + ]) + expect(late.state).toBe(disposed) + expect(late.effects).toEqual([]) +}) + +test("a check deferred while hidden runs on the wake after the timeline is selected", () => { + const run = harness(false) + const id = run.dispatch({ type: "defer", href: "a.ts" }).id ?? 0 + + run.dispatch({ type: "sync", active: true }) + run.dispatch({ type: "wake", epoch: 1 }) + expect(run.effects).toEqual([{ type: "schedule.wake", epoch: 0 }]) + + run.dispatch({ type: "wake", epoch: 0 }) + run.dispatch({ type: "settled", id, found: true }) + expect(run.effects.slice(1)).toEqual([ + { type: "check", id, href: "a.ts" }, + { type: "resolve", id, value: true }, + ]) +}) + +test("a wake from a selection that was hidden again before it ran checks nothing", () => { + const run = harness(false) + + run.dispatch({ type: "defer", href: "a.ts" }) + run.dispatch({ type: "sync", active: true }) + run.dispatch({ type: "sync", active: false }) + const hidden = run.machine.state + + expect(run.dispatch({ type: "wake", epoch: 0 }).state).toBe(hidden) + run.dispatch({ type: "sync", active: true }) + expect(run.effects).toEqual([ + { type: "schedule.wake", epoch: 0 }, + { type: "schedule.wake", epoch: 1 }, + ]) +}) + +test("a negative answer to a check asked before the latest hide is asked again, now or once selected", () => { + const shown = harness() + const again = shown.dispatch({ type: "start", href: "a.ts" }).id ?? 0 + + shown.dispatch({ type: "sync", active: false }) + shown.dispatch({ type: "sync", active: true }) + shown.dispatch({ type: "settled", id: again, found: false }) + shown.dispatch({ type: "settled", id: again, found: false }) + expect(shown.effects).toEqual([ + { type: "check", id: again, href: "a.ts" }, + { type: "resolve", id: again, value: false }, + ]) + + const hidden = harness() + const waits = hidden.dispatch({ type: "start", href: "b.ts" }).id ?? 0 + + hidden.dispatch({ type: "sync", active: false }) + hidden.dispatch({ type: "settled", id: waits, found: false }) + expect(hidden.machine.state).toMatchObject({ status: "hidden", running: [], waiting: [{ id: waits, href: "b.ts" }] }) + expect(hidden.effects).toEqual([]) +}) + +test("a positive answer resolves even while hidden, and a failure rejects once", () => { + const run = harness() + const found = run.dispatch({ type: "start", href: "a.ts" }).id ?? 0 + const broken = run.dispatch({ type: "start", href: "b.ts" }).id ?? 0 + const cause = new Error("offline") + + run.dispatch({ type: "sync", active: false }) + run.dispatch({ type: "settled", id: found, found: true }) + run.dispatch({ type: "failed", id: broken, cause }) + const settled = run.machine.state + + expect(run.dispatch({ type: "failed", id: broken, cause }).state).toBe(settled) + expect(run.effects).toEqual([ + { type: "resolve", id: found, value: true }, + { type: "reject", id: broken, cause }, + ]) +}) diff --git a/packages/app/src/runtime/plugin/link-check-machine.ts b/packages/app/src/runtime/plugin/link-check-machine.ts new file mode 100644 index 000000000000..ec5cbf497cff --- /dev/null +++ b/packages/app/src/runtime/plugin/link-check-machine.ts @@ -0,0 +1,142 @@ +/** A check asked while its timeline was hidden, or one that came back negative while hidden. */ +export type WaitingLinkCheck = { readonly id: number; readonly href: string } + +/** A check whose answer is on its way; `epoch` is the selection it was asked in. */ +export type RunningLinkCheck = { readonly id: number; readonly href: string; readonly epoch: number } + +/** + * Link checks for one timeline. `epoch` counts the times the timeline was hidden, so a negative answer to a check + * asked before the latest hide is a false negative and is asked again. `id` names one caller's request across retries. + */ +export type LinkCheckState = + /** Selected: checks run now. Waiting checks run on the wake scheduled when the timeline was selected. */ + | { + readonly status: "active" + readonly epoch: number + readonly nextId: number + readonly waiting: readonly WaitingLinkCheck[] + readonly running: readonly RunningLinkCheck[] + } + /** Hidden: new checks wait, and a negative answer waits to be asked again. */ + | { + readonly status: "hidden" + readonly epoch: number + readonly nextId: number + readonly waiting: readonly WaitingLinkCheck[] + readonly running: readonly RunningLinkCheck[] + } + /** The timeline's owner went away: every request answered `false`. */ + | { readonly status: "disposed" } + +export type LinkCheckEvent = + | { readonly type: "sync"; readonly active: boolean } + /** The microtask after the timeline was selected in `epoch`, once the routed session has settled. */ + | { readonly type: "wake"; readonly epoch: number } + /** A check asked while hidden. */ + | { readonly type: "defer"; readonly href: string } + /** A check the driver already asked while selected, whose answer is a promise. */ + | { readonly type: "start"; readonly href: string } + | { readonly type: "settled"; readonly id: number; readonly found: boolean } + | { readonly type: "failed"; readonly id: number; readonly cause: unknown } + | { readonly type: "disposed" } + +export type LinkCheckEffect = + | { readonly type: "schedule.wake"; readonly epoch: number } + | { readonly type: "check"; readonly id: number; readonly href: string } + | { readonly type: "resolve"; readonly id: number; readonly value: boolean } + | { readonly type: "reject"; readonly id: number; readonly cause: unknown } + +export type LinkCheckTransition = { + readonly state: LinkCheckState + readonly effects: readonly LinkCheckEffect[] + /** The request a `defer` or `start` event added. */ + readonly id?: number +} + +export function createLinkCheckState(active: boolean): LinkCheckState { + return { status: active ? "active" : "hidden", epoch: 0, nextId: 0, waiting: [], running: [] } +} + +export function transitionLinkCheck(state: LinkCheckState, event: LinkCheckEvent): LinkCheckTransition { + if (state.status === "disposed") return step(state) + + if (event.type === "disposed") + return step( + { status: "disposed" }, + [...state.waiting, ...state.running].map((item) => ({ type: "resolve", id: item.id, value: false })), + ) + + if (event.type === "sync") { + if (event.active === (state.status === "active")) return step(state) + + if (!event.active) return step({ ...state, status: "hidden", epoch: state.epoch + 1 }) + + return step( + { ...state, status: "active" }, + state.waiting.length > 0 ? [{ type: "schedule.wake", epoch: state.epoch }] : [], + ) + } + + if (event.type === "wake") { + if (state.status !== "active" || event.epoch !== state.epoch || state.waiting.length === 0) return step(state) + const woken = state.waiting.map((item) => ({ id: item.id, href: item.href, epoch: state.epoch })) + + return step( + { ...state, waiting: [], running: [...state.running, ...woken] }, + woken.map((item) => ({ type: "check", id: item.id, href: item.href })), + ) + } + + if (event.type === "defer") { + const id = state.nextId + 1 + const wake = state.status === "active" && state.waiting.length === 0 + + return { + state: { ...state, nextId: id, waiting: [...state.waiting, { id, href: event.href }] }, + effects: wake ? [{ type: "schedule.wake", epoch: state.epoch }] : [], + id, + } + } + + if (event.type === "start") { + const id = state.nextId + 1 + + return { + state: { ...state, nextId: id, running: [...state.running, { id, href: event.href, epoch: state.epoch }] }, + effects: [], + id, + } + } + + const entry = state.running.find((item) => item.id === event.id) + + if (!entry) return step(state) + const running = state.running.filter((item) => item !== entry) + + if (event.type === "failed") + return step({ ...state, running }, [{ type: "reject", id: entry.id, cause: event.cause }]) + + if (event.found || (state.status === "active" && entry.epoch === state.epoch)) + return step({ ...state, running }, [{ type: "resolve", id: entry.id, value: event.found }]) + + // A negative answer from a check asked before the timeline was last hidden may be a false negative. + if (state.status === "hidden") + return step({ ...state, running, waiting: [...state.waiting, { id: entry.id, href: entry.href }] }) + + return step({ ...state, running: [...running, { id: entry.id, href: entry.href, epoch: state.epoch }] }, [ + { type: "check", id: entry.id, href: entry.href }, + ]) +} + +export function selectLinkCheckDisposed(state: LinkCheckState) { + return state.status === "disposed" +} + +/** Checks run now only while the timeline is selected. */ +export function selectLinkCheckActive(state: LinkCheckState) { + return state.status === "active" +} + +function step(state: LinkCheckState, effects: readonly LinkCheckEffect[] = []): LinkCheckTransition { + return { state, effects } +} diff --git a/packages/app/src/runtime/plugin/link-check.ts b/packages/app/src/runtime/plugin/link-check.ts index 4998ec675685..382376937f51 100644 --- a/packages/app/src/runtime/plugin/link-check.ts +++ b/packages/app/src/runtime/plugin/link-check.ts @@ -1,35 +1,82 @@ -import { createComputed, untrack, type Accessor } from "solid-js" +import { createComputed, getOwner, onCleanup, untrack, type Accessor } from "solid-js" +import { + createLinkCheckState, + selectLinkCheckActive, + selectLinkCheckDisposed, + transitionLinkCheck, + type LinkCheckEffect, + type LinkCheckEvent, +} from "./link-check-machine" /** * Asks whether a link's file exists only while its timeline is selected. Plugins check links for the selected * session alone, so a hidden timeline's answer would be a false negative: a check asked while hidden, or one that - * comes back negative after the timeline was hidden, even briefly, waits and asks again once it is selected. + * comes back negative after the timeline was hidden, even briefly, waits and asks again once it is selected. Once the + * timeline's owner goes away, every check still waiting answers `false`. */ export function createLinkCheck(active: Accessor, check: (href: string) => boolean | Promise) { - let waiting: (() => void)[] = [] - let hidden = 0 + const machine = { state: createLinkCheckState(untrack(active)) } + const waiters = new Map>() + + const dispatch = (event: LinkCheckEvent) => { + const next = transitionLinkCheck(machine.state, event) + + machine.state = next.state + next.effects.forEach(run) + + return next.id + } + + const run = (effect: LinkCheckEffect) => { + // A microtask after the timeline is selected, so the routed session has settled before the check runs. + if (effect.type === "schedule.wake") return queueMicrotask(() => dispatch({ type: "wake", epoch: effect.epoch })) + + if (effect.type === "check") + return answer( + effect.id, + Promise.try(() => untrack(() => check(effect.href))), + ) + const waiter = waiters.get(effect.id) + waiters.delete(effect.id) + + if (effect.type === "resolve") return waiter?.resolve(effect.value) + waiter?.reject(effect.cause) + } + + const answer = (id: number, result: Promise) => + void result.then( + (found) => dispatch({ type: "settled", id, found }), + (cause: unknown) => dispatch({ type: "failed", id, cause }), + ) + + const wait = (id: number | undefined) => { + if (id === undefined) return false + const waiter = Promise.withResolvers() + waiters.set(id, waiter) + + return waiter.promise + } createComputed(() => { - if (!active()) { - hidden++ + const next = active() - return - } - - const ready = waiting - waiting = [] - ready.forEach((resolve) => resolve()) + untrack(() => dispatch({ type: "sync", active: next })) }) - const exists = (href: string): boolean | Promise => { - if (!untrack(active)) return new Promise((resolve) => waiting.push(resolve)).then(() => exists(href)) - const started = hidden + if (getOwner()) onCleanup(() => dispatch({ type: "disposed" })) + + return (href: string): boolean | Promise => { + if (selectLinkCheckDisposed(machine.state)) return false + + if (!selectLinkCheckActive(machine.state)) return wait(dispatch({ type: "defer", href })) const result = untrack(() => check(href)) if (result === true || result === false) return result + const id = dispatch({ type: "start", href }) + const pending = wait(id) - return result.then((found) => (found || (untrack(active) && hidden === started) ? found : exists(href))) + if (id !== undefined) answer(id, result) + + return pending } - - return exists } diff --git a/packages/app/src/runtime/plugin/located-machine.test.ts b/packages/app/src/runtime/plugin/located-machine.test.ts new file mode 100644 index 000000000000..0dcaa5c7952c --- /dev/null +++ b/packages/app/src/runtime/plugin/located-machine.test.ts @@ -0,0 +1,124 @@ +import { expect, test } from "bun:test" +import { + createLocatedWritesState, + LOCATED_WRITE_MAX_QUEUE, + LOCATED_WRITE_TTL_MS, + selectLocatedDraining, + selectLocatedWaitingKeys, + transitionLocatedWrites, + type LocatedWritesEffect, + type LocatedWritesEvent, +} from "./located-machine" + +/** + * Drives the machine as `createLocatedWrites` does: runs `run` effects at once, so a write can make another write, and + * drains a key one write at a time. Timer effects are recorded. + */ +function harness() { + const log: string[] = [] + const timers: LocatedWritesEffect[] = [] + const machine = { state: createLocatedWritesState() } + + const dispatch = (event: LocatedWritesEvent) => { + const next = transitionLocatedWrites(machine.state, event) + + machine.state = next.state + next.effects.forEach((effect) => { + if (effect.type === "run") return effect.run() + + if (effect.type === "drain") { + while (selectLocatedDraining(machine.state, effect.key)) dispatch({ type: "drain.next", key: effect.key }) + + return + } + + timers.push(effect) + }) + + return next + } + + const write = (key: string, located: boolean, name: string, then?: () => void) => + dispatch({ + type: "write", + key, + located, + run: () => { + log.push(name) + then?.() + }, + }) + + return { machine, log, timers, dispatch, write } +} + +// Before, a write made once the location was known ran at once, ahead of the writes still held for the session, so a +// close ran before the open it followed and left the tab open. +test("a located write runs after the writes held for its session before the watcher drains them", () => { + const run = harness() + + run.write("s1", false, "open") + run.write("s1", true, "close") + + expect(run.log).toEqual(["open", "close"]) + expect(selectLocatedWaitingKeys(run.machine.state)).toEqual([]) +}) + +// Before, a write made by a held write ran at once, ahead of the writes still held behind it. +test("a write made by a held write joins the end of its session's queue", () => { + const run = harness() + + run.write("s1", false, "A", () => run.write("s1", true, "nested")) + run.write("s1", false, "B") + run.write("s2", false, "other") + run.dispatch({ type: "drain.start", key: "s1" }) + + expect(run.log).toEqual(["A", "B", "nested"]) + expect(selectLocatedWaitingKeys(run.machine.state)).toEqual(["s2"]) +}) + +// Before, a write for a session that never located was kept forever and replayed whenever it did. +test("a held write expires after the TTL and never runs", () => { + const run = harness() + const id = run.machine.state.status === "active" ? run.machine.state.nextId + 1 : 0 + + run.write("s1", false, "open") + run.dispatch({ type: "expire", id }) + run.dispatch({ type: "drain.start", key: "s1" }) + + expect(run.timers).toEqual([{ type: "schedule.expire", id, timeoutMs: LOCATED_WRITE_TTL_MS }]) + expect(run.log).toEqual([]) +}) + +test("a full queue drops its oldest write and that write's timer", () => { + const run = harness() + + Array.from({ length: LOCATED_WRITE_MAX_QUEUE + 1 }, (_, index) => run.write("s1", false, `w${index}`)) + run.dispatch({ type: "drain.start", key: "s1" }) + + expect(run.log).toHaveLength(LOCATED_WRITE_MAX_QUEUE) + expect(run.log[0]).toBe("w1") + expect(run.timers).toContainEqual({ type: "cancel.expire", ids: [1] }) +}) + +test("disposing cancels every held write's timer, and later writes never run", () => { + const run = harness() + + run.write("s1", false, "A") + run.write("s2", false, "B") + run.dispatch({ type: "disposed" }) + const disposed = run.machine.state + + expect(run.write("s1", true, "late").state).toBe(disposed) + expect(run.timers.at(-1)).toEqual({ type: "cancel.expire", ids: [1, 2] }) + expect(run.log).toEqual([]) +}) + +test("a located write with nothing held runs at once without a new state", () => { + const run = harness() + const before = run.machine.state + const next = run.write("s1", true, "now") + + expect(next.state).toBe(before) + expect(run.log).toEqual(["now"]) +}) diff --git a/packages/app/src/runtime/plugin/located-machine.ts b/packages/app/src/runtime/plugin/located-machine.ts new file mode 100644 index 000000000000..2a183b029732 --- /dev/null +++ b/packages/app/src/runtime/plugin/located-machine.ts @@ -0,0 +1,125 @@ +/** A write held for a session whose location stays unknown this long is dropped, so it cannot ambush a later visit. */ +export const LOCATED_WRITE_TTL_MS = 30_000 + +/** Held writes beyond this drop the oldest, so a session that never locates cannot grow the queue without bound. */ +export const LOCATED_WRITE_MAX_QUEUE = 256 + +export type HeldLocatedWrite = { readonly id: number; readonly key: string; readonly run: () => void } + +/** + * Layout writes held until their session's location is known, by session key. A session's writes run in call order: + * while any of its writes is held or `draining`, a new write for it joins the end of the queue, even once it is located. + */ +export type LocatedWritesState = + | { + readonly status: "active" + readonly nextId: number + /** Session keys whose held writes are running now, one at a time. */ + readonly draining: readonly string[] + readonly queue: readonly HeldLocatedWrite[] + } + /** The owner went away: held writes are dropped and new ones never run. */ + | { readonly status: "disposed" } + +export type LocatedWritesEvent = + | { readonly type: "write"; readonly key: string; readonly located: boolean; readonly run: () => void } + /** The session's location became known. */ + | { readonly type: "drain.start"; readonly key: string } + /** Runs the session's next held write, or ends its drain. */ + | { readonly type: "drain.next"; readonly key: string } + | { readonly type: "expire"; readonly id: number } + | { readonly type: "disposed" } + +export type LocatedWritesEffect = + | { readonly type: "run"; readonly run: () => void } + | { readonly type: "drain"; readonly key: string } + | { readonly type: "schedule.expire"; readonly id: number; readonly timeoutMs: number } + | { readonly type: "cancel.expire"; readonly ids: readonly number[] } + +export type LocatedWritesTransition = { + readonly state: LocatedWritesState + readonly effects: readonly LocatedWritesEffect[] +} + +export function createLocatedWritesState(): LocatedWritesState { + return { status: "active", nextId: 0, draining: [], queue: [] } +} + +export function transitionLocatedWrites(state: LocatedWritesState, event: LocatedWritesEvent): LocatedWritesTransition { + if (state.status === "disposed") return step(state) + + if (event.type === "disposed") + return step( + { status: "disposed" }, + state.queue.length > 0 ? [{ type: "cancel.expire", ids: state.queue.map((item) => item.id) }] : [], + ) + + if (event.type === "write") { + const draining = state.draining.includes(event.key) + const held = state.queue.some((item) => item.key === event.key) + + if (event.located && !draining && !held) return step(state, [{ type: "run", run: event.run }]) + const id = state.nextId + 1 + const overflow = state.queue.length >= LOCATED_WRITE_MAX_QUEUE ? state.queue.slice(0, 1) : [] + const queue = [...state.queue.slice(overflow.length), { id, key: event.key, run: event.run }] + + const evicted: LocatedWritesEffect[] = + overflow.length > 0 ? [{ type: "cancel.expire", ids: overflow.map((item) => item.id) }] : [] + + if (!event.located) + return step({ ...state, nextId: id, queue }, [ + ...evicted, + { type: "schedule.expire", id, timeoutMs: LOCATED_WRITE_TTL_MS }, + ]) + + if (draining) return step({ ...state, nextId: id, queue }, evicted) + + // Located before the location watcher ran: the writes held before it run first, in order. + return step({ ...state, nextId: id, queue, draining: [...state.draining, event.key] }, [ + ...evicted, + { type: "drain", key: event.key }, + ]) + } + + if (event.type === "drain.start") { + if (state.draining.includes(event.key) || !state.queue.some((item) => item.key === event.key)) return step(state) + + return step({ ...state, draining: [...state.draining, event.key] }, [{ type: "drain", key: event.key }]) + } + + if (event.type === "drain.next") { + if (!state.draining.includes(event.key)) return step(state) + const head = state.queue.find((item) => item.key === event.key) + + if (!head) return step({ ...state, draining: state.draining.filter((key) => key !== event.key) }) + + return step({ ...state, queue: state.queue.filter((item) => item !== head) }, [ + { type: "cancel.expire", ids: [head.id] }, + { type: "run", run: head.run }, + ]) + } + + if (!state.queue.some((item) => item.id === event.id)) return step(state) + + return step({ ...state, queue: state.queue.filter((item) => item.id !== event.id) }) +} + +/** Whether a write for `key` would wait behind earlier ones. */ +export function selectLocatedSessionBusy(state: LocatedWritesState, key: string) { + return state.status === "active" && (state.draining.includes(key) || state.queue.some((item) => item.key === key)) +} + +export function selectLocatedDraining(state: LocatedWritesState, key: string) { + return state.status === "active" && state.draining.includes(key) +} + +/** Session keys with held writes that wait for a location, in first-held order. */ +export function selectLocatedWaitingKeys(state: LocatedWritesState): readonly string[] { + if (state.status === "disposed") return [] + + return Array.from(new Set(state.queue.flatMap((item) => (state.draining.includes(item.key) ? [] : [item.key])))) +} + +function step(state: LocatedWritesState, effects: readonly LocatedWritesEffect[] = []): LocatedWritesTransition { + return { state, effects } +} diff --git a/packages/app/src/runtime/plugin/located.ts b/packages/app/src/runtime/plugin/located.ts index 6b44f58d2e6f..12110435865d 100644 --- a/packages/app/src/runtime/plugin/located.ts +++ b/packages/app/src/runtime/plugin/located.ts @@ -1,28 +1,82 @@ -import { batch, createEffect, createSignal, untrack } from "solid-js" +import { batch, createEffect, createMemo, createSignal, getOwner, onCleanup, untrack } from "solid-js" import type { SessionRef } from "@opencode/plugin/gui" +import { same } from "@/runtime/persistence/equality" +import { + createLocatedWritesState, + selectLocatedDraining, + selectLocatedSessionBusy, + selectLocatedWaitingKeys, + transitionLocatedWrites, + type LocatedWritesEvent, +} from "./located-machine" /** * Layout writes find a session's stored layout through its location, which is unknown until the server reports it - * (for example after a server re-authenticates). `hold` keeps such a write and runs it, in order, once the location is - * known, so no write is lost. + * (for example after a server re-authenticates). `hold` keeps such a write and runs it once the location is known; a + * session's writes always run in call order. A write whose session stays unlocated for `LOCATED_WRITE_TTL_MS` is + * dropped. */ export function createLocatedWrites() { - const [pending, setPending] = createSignal void }[]>([]) - // Syncs held writes with the session locations the server reports. + const [writesState, setWritesState] = createSignal(createLocatedWritesState()) + // The latest ref per session with held writes, whose location the watcher below reads. + const refs = new Map() + const timers = new Map>() + const waiting = createMemo(() => selectLocatedWaitingKeys(writesState()), [], { equals: same }) + + const dispatch = (event: LocatedWritesEvent) => { + const next = transitionLocatedWrites(untrack(writesState), event) + + setWritesState(next.state) + next.effects.forEach((effect) => { + // Promise.try runs each write synchronously and isolates a throw from the writes behind it. + if (effect.type === "run") + return void Promise.try(effect.run).catch((cause: unknown) => console.error("[plugin]", cause)) + + // Each step takes the next write off the queue before running it, and a write it makes joins the end. + if (effect.type === "drain") + return batch(() => { + while (selectLocatedDraining(untrack(writesState), effect.key)) + dispatch({ type: "drain.next", key: effect.key }) + }) + + if (effect.type === "schedule.expire") + return void timers.set( + effect.id, + setTimeout(() => { + timers.delete(effect.id) + dispatch({ type: "expire", id: effect.id }) + }, effect.timeoutMs), + ) + + effect.ids.forEach((id) => { + clearTimeout(timers.get(id)) + timers.delete(id) + }) + }) + } + + // Drains each session whose location the server has reported. createEffect(() => { - const held = pending() + const keys = waiting() + Array.from(refs.keys()).forEach((key) => { + if (!untrack(() => selectLocatedSessionBusy(writesState(), key))) refs.delete(key) + }) + const ready = keys.filter((key) => refs.get(key)?.location) - if (held.length === 0) return - const located = held.filter((item) => item.session.location) - - if (located.length === 0) return - setPending(held.filter((item) => !located.includes(item))) - untrack(() => batch(() => located.forEach((item) => item.run()))) + if (ready.length === 0) return + untrack(() => batch(() => ready.forEach((key) => dispatch({ type: "drain.start", key })))) }) + if (getOwner()) onCleanup(() => dispatch({ type: "disposed" })) + return { hold(session: SessionRef, run: () => void) { - setPending((held) => [...held, { session, run }]) + const located = Boolean(untrack(() => session.location)) + + if (!located || selectLocatedSessionBusy(untrack(writesState), session.key)) refs.set(session.key, session) + dispatch({ type: "write", key: session.key, located, run }) }, + /** Whether a write for the session would wait behind earlier ones. */ + pending: (key: string) => selectLocatedSessionBusy(untrack(writesState), key), } } diff --git a/packages/app/test-browser/plugin-link-check.test.ts b/packages/app/test-browser/plugin-link-check.test.ts index ae58331f1b25..881383e547c8 100644 --- a/packages/app/test-browser/plugin-link-check.test.ts +++ b/packages/app/test-browser/plugin-link-check.test.ts @@ -66,6 +66,23 @@ test("asks again when a check comes back negative after the timeline was hidden" } }) +// Before, a check waiting on a hidden timeline never settled once the timeline was disposed, and a negative answer +// arriving after it queued yet another wait. +test("answers false to every check still waiting or running when the timeline is disposed", async () => { + const answer = Promise.withResolvers() + const input = setup((href) => (href === "running.ts" ? answer.promise : true)) + const running = input.exists("running.ts") + + input.setActive(false) + const waiting = input.exists("waiting.ts") + input.dispose() + answer.resolve(false) + + expect(await Promise.all([running, waiting])).toEqual([false, false]) + expect(input.exists("late.ts")).toBe(false) + expect(input.asked).toEqual(["running.ts"]) +}) + test("asks again when a check comes back negative after the timeline was hidden and shown again", async () => { const answers = [Promise.withResolvers(), Promise.withResolvers()] const input = setup(() => answers[input.asked.length - 1]!.promise) diff --git a/packages/app/test-browser/plugin-primitives.test.ts b/packages/app/test-browser/plugin-primitives.test.ts index 47731f240d40..1f304e19fb4b 100644 --- a/packages/app/test-browser/plugin-primitives.test.ts +++ b/packages/app/test-browser/plugin-primitives.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test } from "bun:test" +import { describe, expect, jest, test } from "bun:test" import { batch, createComponent, createRoot, createSignal, onCleanup, type Accessor } from "solid-js" import { Schema } from "effect" import { @@ -14,6 +14,7 @@ import type { Platform } from "@/runtime/platform/platform" import { flushPersisted } from "@/runtime/persistence/persist" import { Persist, persisted, removePersisted } from "@/runtime/persistence/storage" import { createLocatedWrites } from "@/runtime/plugin/located" +import { LOCATED_WRITE_TTL_MS } from "@/runtime/plugin/located-machine" import { createSessionStore, globalStoreTarget, persistedHandle, whenLoaded } from "@/runtime/plugin/stores" const pending = { status: "pending" } as const @@ -591,4 +592,25 @@ describe("plugin primitives", () => { expect({ before, after: log }).toEqual({ before: [], after: ["open", "scroll"] }) root.dispose() }) + + // Before, a write held for a session that never located was kept forever and replayed whenever it did. + test("a held write expires when the session stays unlocated past the TTL", () => { + jest.useFakeTimers() + const [location, setLocation] = createSignal<{ directory: string } | undefined>() + const target = session(() => "server\nses_gone", location) + const log: string[] = [] + const root = createRoot((dispose) => ({ dispose, writes: createLocatedWrites() })) + + try { + root.writes.hold(target, () => log.push("open")) + jest.advanceTimersByTime(LOCATED_WRITE_TTL_MS) + setLocation({ directory: "/repo" }) + root.writes.hold(target, () => log.push("later")) + + expect(log).toEqual(["later"]) + } finally { + root.dispose() + jest.useRealTimers() + } + }) })