diff --git a/packages/app/e2e/regression/terminal.spec.ts b/packages/app/e2e/regression/terminal.spec.ts index 0495ae0cf1e..762fd293853 100644 --- a/packages/app/e2e/regression/terminal.spec.ts +++ b/packages/app/e2e/regression/terminal.spec.ts @@ -189,6 +189,29 @@ test("a click into the composer while a new terminal is created keeps the focus await expect(editor).toBeFocused() }) +// Both presses used to send a create, so two tabs both titled "Terminal 1" opened. +test("pressing New terminal again while one is created creates one terminal", async ({ page }) => { + const { persistentPty, editor } = await openSession(page, { name: "Terminals", persistentPty: {} }) + + const create = await holdRoute(page, /\/api\/experimental\/session\/ses_terminals\/terminal(\?|$)/, { + method: "POST", + }) + + await editor.click() + await page.keyboard.press("Control+Alt+T") + await create.arrived + await page.keyboard.press("Control+Alt+T") + create.release() + + await expect(side(page).getByRole("tab", { name: "Terminal 1", exact: true })).toHaveAttribute( + "aria-selected", + "true", + ) + await expect(terminalScreen(page, persistentPty.created[0]!.id).locator("textarea")).toBeFocused() + expect(create.requests).toHaveLength(1) + expect(persistentPty.created.map((info) => info.title)).toEqual(["Terminal 1"]) +}) + test("another client's control ends at a click, which resizes the terminal to the pane", async ({ page }) => { const { persistentPty } = await openSession(page, { name: "Terminals", @@ -288,6 +311,37 @@ test("a terminal list that fails says why, and Try again lists the terminals", a await expect(side(page).getByText("Couldn't load terminals", { exact: true })).toHaveCount(0) }) +// A list the server accepted and never answered used to leave "Loading terminal." on screen with no way to retry. +test("a terminal list the server never answers says so, and Try again lists the terminals", async ({ page }) => { + await openSession(page, { + name: "Terminals", + persistentPty: { initial: [{ id: "pty_build", sessionID: SESSION, title: "Build", output: "still-here\r\n" }] }, + }) + + await openSide(page) + await side(page).getByRole("tab", { name: "Build", exact: true }).click() + const screen = terminalScreen(page, "pty_build") + await expect.poll(() => terminalText(page, screen)).toBe("still-here") + + const list = await holdRoute(page, /\/api\/experimental\/session\/ses_terminals\/terminal(\?|$)/, { method: "GET" }) + await page.clock.install() + await page.reload() + await list.arrived + await expect(side(page).getByText("Loading terminal…", { exact: true })).toBeVisible() + // The request budget, `TERMINAL_REQUEST_TIMEOUT_MS`. + await page.clock.fastForward(15_000) + + const failure = side(page).getByRole("alert") + await expect(failure.getByText("Couldn't load terminals", { exact: true })).toBeVisible() + await expect(failure.getByText("The server took too long to answer", { exact: true })).toBeVisible() + + list.release() + await failure.getByRole("button", { name: "Try again", exact: true }).click() + + await expect(side(page).getByRole("tab", { name: "Build", exact: true })).toHaveAttribute("aria-selected", "true") + await expect.poll(() => terminalText(page, screen)).toBe("still-here") +}) + test("after a server restart, showing the terminal again restores it from a new snapshot", async ({ page }) => { const output = "before-restart\r\n" diff --git a/packages/gui-plugins/README.md b/packages/gui-plugins/README.md index 50f88b90e5c..d3a685c55af 100644 --- a/packages/gui-plugins/README.md +++ b/packages/gui-plugins/README.md @@ -391,12 +391,9 @@ The updater's actions branch on the state its main entry pushes and still answer ```tsx -const act = (name: "check" | "install") => { - if (state()) return void import("./actions").then((module) => module[name](ctx, updater)) - - // Not loaded yet, or not active (failed, or not on this platform): nothing can check or install. - showToast({ title: ctx.t("common.requestFailed") }) -} +// Main missing (not loaded yet, failed, or not on this platform) answers that nothing can check or install. +const act = (name: "check" | "install") => + dispatchWindow({ type: "action.requested", action: name, available: state() !== undefined }) ``` The pairing settings page waits for its main side only where it needs it. Pairing codes come from the local server's `ServerRef`, so they never wait for main; remote access and the display setting render only while main is active, so the page never offers a switch that cannot answer: diff --git a/packages/gui-plugins/src/browser/i18n/en.ts b/packages/gui-plugins/src/browser/i18n/en.ts index 65fd4d1bcc5..b33085d6a27 100644 --- a/packages/gui-plugins/src/browser/i18n/en.ts +++ b/packages/gui-plugins/src/browser/i18n/en.ts @@ -1,4 +1,5 @@ export default { + "model.timedOut": "The browser took too long to load", "command.open": "Open browser", "command.reload": "Reload browser page", "command.inspect": "Select an element in the browser page", diff --git a/packages/gui-plugins/src/browser/model-machine.test.ts b/packages/gui-plugins/src/browser/model-machine.test.ts new file mode 100644 index 00000000000..a55ea82e428 --- /dev/null +++ b/packages/gui-plugins/src/browser/model-machine.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, test } from "bun:test" +import { + createBrowserModelState, + selectBrowserModel, + selectBrowserModelError, + selectBrowserTabHidden, + transitionBrowserModel, + type BrowserModelEffect, + type BrowserModelEvent, + type BrowserModelState, +} from "./model-machine" + +type Model = { readonly name: string } + +const replay = (events: readonly BrowserModelEvent[], state = createBrowserModelState()) => + events.reduce<{ state: BrowserModelState; effects: BrowserModelEffect[] }>( + (last, event) => { + const next = transitionBrowserModel(last.state, event) + + return { state: next.state, effects: [...last.effects, ...next.effects] } + }, + { state, effects: [] }, + ) + +const known = () => true + +describe("browser model machine", () => { + // A model chunk that failed to load used to leave the panel blank, its stored tabs hidden and its commands disabled + // for the rest of the run. + test("a load that fails shows its error and the stored tabs, and a retry loads again", () => { + const failed = replay([ + { type: "load.requested" }, + { type: "load.failed", attempt: 1, error: "chunk" }, + { type: "load.requested" }, + ]) + + expect(selectBrowserModelError(failed.state)).toBe("chunk") + expect(selectBrowserTabHidden(failed.state, known)).toBe(false) + expect(failed.effects).toEqual([{ type: "load-model", attempt: 1 }]) + + const retried = replay( + [{ type: "retry.requested" }, { type: "load.succeeded", attempt: 2, model: { name: "m" } }], + failed.state, + ) + + expect(retried.effects).toEqual([{ type: "load-model", attempt: 2 }]) + expect(selectBrowserModel(retried.state)).toEqual({ name: "m" }) + expect(selectBrowserTabHidden(retried.state, () => false)).toBe(true) + }) + + test("stored tabs stay hidden while the model loads, and a second request loads once", () => { + const loading = replay([{ type: "load.requested" }, { type: "load.requested" }, { type: "retry.requested" }]) + + expect(loading.effects).toEqual([{ type: "load-model", attempt: 1 }]) + expect(selectBrowserTabHidden(loading.state, known)).toBe(true) + }) + + test("a load the plugin's cleanup or a newer attempt overtook settles nothing", () => { + const disposed = replay([ + { type: "load.requested" }, + { type: "disposed" }, + { type: "load.succeeded", attempt: 1, model: { name: "late" } }, + { type: "retry.requested" }, + ]) + + expect(disposed.state).toEqual({ status: "disposed" }) + + const stale = replay([ + { type: "load.requested" }, + { type: "load.failed", attempt: 1, error: "chunk" }, + { type: "retry.requested" }, + { type: "load.failed", attempt: 1, error: "late" }, + ]) + + expect(stale.state).toEqual({ status: "loading", attempt: 2 }) + }) +}) diff --git a/packages/gui-plugins/src/browser/model-machine.ts b/packages/gui-plugins/src/browser/model-machine.ts new file mode 100644 index 00000000000..d45714a703a --- /dev/null +++ b/packages/gui-plugins/src/browser/model-machine.ts @@ -0,0 +1,92 @@ +// The window's browser model, loaded when the first session opens. Each load takes the next attempt number, so a load +// the plugin's cleanup or a retry overtook never hands out its model. + +/** How long the model's chunk may take to load before the panel offers a retry. */ +export const BROWSER_MODEL_LOAD_TIMEOUT_MS = 15_000 + +/** + * - `unloaded`: no session opened yet; `issued` is the last attempt handed out. + * - `loading`: the model's chunk loads. + * - `loaded`: the model serves the panel, its tabs and commands. + * - `failed`: the chunk failed to load or ran out of time; the panel shows why, with a retry. + * - `disposed`: the plugin instance went away. + */ +export type BrowserModelState = + | { readonly status: "unloaded"; readonly issued: number } + | { readonly status: "loading"; readonly attempt: number } + | { readonly status: "loaded"; readonly model: M } + | { readonly status: "failed"; readonly issued: number; readonly error: string } + | { readonly status: "disposed" } + +export type BrowserModelEvent = + /** The first session opened. */ + | { readonly type: "load.requested" } + /** The user asks to load it again after a failure. */ + | { readonly type: "retry.requested" } + | { readonly type: "load.succeeded"; readonly attempt: number; readonly model: M } + /** The chunk failed to load or ran out of time. */ + | { readonly type: "load.failed"; readonly attempt: number; readonly error: string } + | { readonly type: "disposed" } + +export type BrowserModelEffect = { readonly type: "load-model"; readonly attempt: number } + +export type BrowserModelTransition = { + readonly state: BrowserModelState + readonly effects: readonly BrowserModelEffect[] +} + +export const createBrowserModelState = (): BrowserModelState => ({ status: "unloaded", issued: 0 }) + +export function transitionBrowserModel( + state: BrowserModelState, + event: BrowserModelEvent, +): BrowserModelTransition { + if (state.status === "disposed") return unchanged(state) + + if (event.type === "disposed") return unchanged({ status: "disposed" }) + + if ( + (event.type === "load.requested" && state.status === "unloaded") || + (event.type === "retry.requested" && state.status === "failed") + ) { + const attempt = state.issued + 1 + + return { state: { status: "loading", attempt }, effects: [{ type: "load-model", attempt }] } + } + + if (event.type === "load.succeeded" && isLoadingBrowserModel(state, event.attempt)) + return unchanged({ status: "loaded", model: event.model }) + + if (event.type === "load.failed" && isLoadingBrowserModel(state, event.attempt)) + return unchanged({ status: "failed", issued: event.attempt, error: event.error }) + + return unchanged(state) +} + +/** Whether a load that just imported its module should still create the model. */ +export function isLoadingBrowserModel(state: BrowserModelState, attempt: number) { + return state.status === "loading" && state.attempt === attempt +} + +export function selectBrowserModel(state: BrowserModelState) { + return state.status === "loaded" ? state.model : undefined +} + +/** Why the model failed to load, while the panel offers a retry. */ +export function selectBrowserModelError(state: BrowserModelState) { + return state.status === "failed" ? state.error : undefined +} + +/** + * A stored tab stays listed but hidden until the model knows it. Once the load failed it shows, so the panel can say + * why and offer a retry. + */ +export function selectBrowserTabHidden(state: BrowserModelState, known: (model: M) => boolean) { + if (state.status === "failed") return false + + return state.status !== "loaded" || !known(state.model) +} + +function unchanged(state: BrowserModelState): BrowserModelTransition { + return { state, effects: [] } +} diff --git a/packages/gui-plugins/src/browser/renderer.tsx b/packages/gui-plugins/src/browser/renderer.tsx index 50a870d796f..c4121a17c39 100644 --- a/packages/gui-plugins/src/browser/renderer.tsx +++ b/packages/gui-plugins/src/browser/renderer.tsx @@ -1,9 +1,32 @@ -import { createMemo, createSignal, getOwner, lazy, onCleanup, runWithOwner, Show, Suspense } from "solid-js" +import { + createMemo, + createSignal, + getOwner, + lazy, + Match, + onCleanup, + runWithOwner, + Show, + Suspense, + Switch, + untrack, +} from "solid-js" +import { Button } from "@opencode/ui/button" import { Command, createKeyed, LinkHandler, MenuItem, Style, type Plugin } from "@opencode/plugin/gui" import { BuiltinCommand, BuiltinPanel, onIdle, type BuiltinPanelTab } from "@opencode/plugin/gui/builtin" import { Browser } from "@opencode/plugin/gui/browser" import type definition from "./index" import type { Model } from "./model" +import { + BROWSER_MODEL_LOAD_TIMEOUT_MS, + createBrowserModelState, + isLoadingBrowserModel, + selectBrowserModel, + selectBrowserModelError, + selectBrowserTabHidden, + transitionBrowserModel, + type BrowserModelEvent, +} from "./model-machine" import { PageIcon } from "./page-icon" import barStyles from "./bar.css?inline" import commentStyles from "./comment.css?inline" @@ -11,7 +34,8 @@ import tabStyles from "./tabs.css?inline" const setup: Plugin.Setup = (ctx) => { const sessions = ctx.sessions - const [model, setModel] = createSignal() + const [modelState, setModelState] = createSignal(createBrowserModelState()) + const model = () => selectBrowserModel(modelState()) const mac = /(Mac|iPhone|iPad)/.test(navigator.platform) // Settings > Shortcuts lists the command on every platform; it stays disabled until the pane can open. Bound like // Review's and Open file's launchers, since the sidebar owns mod+shift+b and mod+t opens a tab inside the pane. @@ -40,15 +64,42 @@ const setup: Plugin.Setup = (ctx) => { ctx.add(Style, barStyles) // Everything here serves a mounted session, so the attachment model and the pane's protocol // schemas load when the first session opens instead of at startup. + const owner = getOwner() + + const dispatchModel = (event: BrowserModelEvent) => { + const next = transitionBrowserModel(untrack(modelState), event) + + if (next.state !== untrack(modelState)) setModelState(() => next.state) + next.effects.forEach((effect) => loadModel(effect.attempt)) + } + + // A chunk that fails or never loads, such as after an update replaced the app's files, ends in `failed`. + const loadModel = (attempt: number) => { + const deadline = Promise.withResolvers() + const timer = setTimeout(() => deadline.reject(new Error(ctx.t("model.timedOut"))), BROWSER_MODEL_LOAD_TIMEOUT_MS) + + void Promise.race([import("./model"), deadline.promise]) + .finally(() => clearTimeout(timer)) + .then( + (module) => { + if (ctx.signal.aborted || !isLoadingBrowserModel(untrack(modelState), attempt)) return + const created = runWithOwner(owner, () => module.createModel(ctx))! + + dispatchModel({ type: "load.succeeded", attempt, model: created }) + }, + (cause: unknown) => + dispatchModel({ + type: "load.failed", + attempt, + error: cause instanceof Error ? cause.message : String(cause), + }), + ) + } + + onCleanup(() => dispatchModel({ type: "disposed" })) + const opened = createMemo((seen: boolean) => seen || !!sessions.current(), false) - createKeyed(opened, () => { - const owner = getOwner() - void import("./model").then((module) => { - if (ctx.signal.aborted) return - const created = runWithOwner(owner, () => module.createModel(ctx)) - setModel(() => created) - }) - }) + createKeyed(opened, () => dispatchModel({ type: "load.requested" })) ctx.provide(Browser, { attached: (session) => model()?.attached(session) ?? false, @@ -224,9 +275,10 @@ const setup: Plugin.Setup = (ctx) => { {text()} ), - // Listed only to hold the restored selection until the desktop's first inventory names the tab. + // Listed only to hold the restored selection until the desktop's first inventory names the tab, or the model + // failed to load and the panel says so. get hidden() { - return !model()?.tab({ key: session }, id) + return selectBrowserTabHidden(modelState(), (value) => !!value.tab({ key: session }, id)) }, group: "browser", dom: { tab: `session-side-panel-browser-tab-${id}`, panel: "session-side-panel-browser-tabpanel" }, @@ -256,15 +308,35 @@ const setup: Plugin.Setup = (ctx) => { }, // The pane shows nothing until the desktop's first inventory names its tabs. render: (props) => ( - - {(value) => ( - - - props.tab} session={props.session} screen={props.screen} model={value()} /> - - - )} - + + + {(value) => ( + + + props.tab} + session={props.session} + screen={props.screen} + model={value()} + /> + + + )} + + + {(error) => ( + + )} + + ), close: (input) => model()?.closeTab(input.session, input.tab.id), // A blank new tab does not come back; a page comes back as a new tab on its address. diff --git a/packages/gui-plugins/src/ssh/add-machine.test.ts b/packages/gui-plugins/src/ssh/add-machine.test.ts new file mode 100644 index 00000000000..df28bb53595 --- /dev/null +++ b/packages/gui-plugins/src/ssh/add-machine.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, test } from "bun:test" +import { initialSshAdd, transitionSshAdd, type SshAddEffect, type SshAddEvent, type SshAddState } from "./add-machine" + +type Module = { readonly name: string } + +const replay = (events: readonly SshAddEvent[]) => + events.reduce<{ state: SshAddState; effects: SshAddEffect[] }>( + (last, event) => { + const next = transitionSshAdd(last.state, event) + + return { state: next.state, effects: [...last.effects, ...next.effects] } + }, + { state: initialSshAdd, effects: [] }, + ) + +const module: Module = { name: "dialog" } + +describe("ssh add dialog machine", () => { + // A double click used to load the chunk twice and open two dialogs. + test("a second click joins the dialog loading and opens one", () => { + const result = replay([ + { type: "add.requested", openProject: false }, + { type: "add.requested", openProject: true }, + { type: "add.loaded", attempt: 1, module }, + { type: "add.loaded", attempt: 1, module }, + ]) + + expect(result.effects).toEqual([ + { type: "load", attempt: 1 }, + { type: "open", module, openProject: true }, + ]) + }) + + // A chunk that failed to load used to leave the click unanswered, with an unhandled rejection. + test("a chunk that fails or stalls says so and the next click loads again", () => { + const result = replay([ + { type: "add.requested", openProject: false }, + { type: "add.failed", attempt: 1, error: "chunk" }, + { type: "add.requested", openProject: false }, + ]) + + expect(result.effects).toEqual([ + { type: "load", attempt: 1 }, + { type: "toast-failed", error: "chunk" }, + { type: "load", attempt: 2 }, + ]) + }) + + test("a chunk that loads after the plugin went away opens nothing", () => { + const result = replay([ + { type: "add.requested", openProject: false }, + { type: "disposed" }, + { type: "add.loaded", attempt: 1, module }, + { type: "add.requested", openProject: false }, + ]) + + expect(result.state).toEqual({ status: "disposed" }) + expect(result.effects).toEqual([{ type: "load", attempt: 1 }]) + }) +}) diff --git a/packages/gui-plugins/src/ssh/add-machine.ts b/packages/gui-plugins/src/ssh/add-machine.ts new file mode 100644 index 00000000000..b5aa0e0b871 --- /dev/null +++ b/packages/gui-plugins/src/ssh/add-machine.ts @@ -0,0 +1,63 @@ +// The "Add SSH server" dialog, whose chunk loads on the first click. Each load takes the next attempt number, so a load +// the plugin's cleanup overtook never opens its dialog. + +/** How long the dialog's chunk may take to load before the click fails. */ +export const SSH_DIALOG_LOAD_TIMEOUT_MS = 15_000 + +/** + * - `idle`: no dialog loading; `issued` is the last attempt handed out. + * - `loading`: the chunk loads; a second click joins it. `openProject` opens the server's project once it connects. + * - `disposed`: the plugin instance went away. + */ +export type SshAddState = + | { readonly status: "idle"; readonly issued: number } + | { readonly status: "loading"; readonly attempt: number; readonly openProject: boolean } + | { readonly status: "disposed" } + +export type SshAddEvent = + | { readonly type: "add.requested"; readonly openProject: boolean } + | { readonly type: "add.loaded"; readonly attempt: number; readonly module: M } + /** The chunk failed to load or ran out of time. */ + | { readonly type: "add.failed"; readonly attempt: number; readonly error: string } + | { readonly type: "disposed" } + +export type SshAddEffect = + | { readonly type: "load"; readonly attempt: number } + | { readonly type: "open"; readonly module: M; readonly openProject: boolean } + | { readonly type: "toast-failed"; readonly error: string } + +export type SshAddTransition = { readonly state: SshAddState; readonly effects: readonly SshAddEffect[] } + +export const initialSshAdd: SshAddState = { status: "idle", issued: 0 } + +export function transitionSshAdd(state: SshAddState, event: SshAddEvent): SshAddTransition { + if (state.status === "disposed") return unchanged(state) + + if (event.type === "disposed") return unchanged({ status: "disposed" }) + + if (event.type === "add.requested") { + // A second click joins the dialog already loading; the command's project choice wins over the menu's. + if (state.status === "loading") + return unchanged(event.openProject && !state.openProject ? { ...state, openProject: true } : state) + const attempt = state.issued + 1 + + return { + state: { status: "loading", attempt, openProject: event.openProject }, + effects: [{ type: "load", attempt }], + } + } + + if (state.status !== "loading" || event.attempt !== state.attempt) return unchanged(state) + + if (event.type === "add.loaded") + return { + state: { status: "idle", issued: state.attempt }, + effects: [{ type: "open", module: event.module, openProject: state.openProject }], + } + + return { state: { status: "idle", issued: state.attempt }, effects: [{ type: "toast-failed", error: event.error }] } +} + +function unchanged(state: SshAddState): SshAddTransition { + return { state, effects: [] } +} diff --git a/packages/gui-plugins/src/ssh/renderer.tsx b/packages/gui-plugins/src/ssh/renderer.tsx index 8932ff69f2e..696c6efe530 100644 --- a/packages/gui-plugins/src/ssh/renderer.tsx +++ b/packages/gui-plugins/src/ssh/renderer.tsx @@ -1,5 +1,5 @@ import { showToast } from "@opencode/ui/toast" -import { createRoot, lazy, onCleanup, Suspense, type JSX } from "solid-js" +import { createRoot, createSignal, lazy, onCleanup, Suspense, untrack, type JSX } from "solid-js" import { Command, createKeyed, MenuItem, Server, Style, type ServerEntry, type Plugin } from "@opencode/plugin/gui" import { onIdle } from "@opencode/plugin/gui/builtin" import { Ssh, type SshConfig, type SshItem } from "./contract" @@ -7,9 +7,18 @@ import { SshCover } from "./cover" import type definition from "./index" import { sshName, sshServerState } from "./name" import { createSshController } from "./state" +import { + initialSshAdd, + SSH_DIALOG_LOAD_TIMEOUT_MS, + transitionSshAdd, + type SshAddEffect, + type SshAddEvent, +} from "./add-machine" const loadDialog = () => import("./dialog") +type DialogModule = Awaited> + const setup: Plugin.Setup = (ctx) => { // SSH lives in the desktop main process. if (ctx.app.platform !== "desktop") return @@ -57,24 +66,54 @@ const setup: Plugin.Setup = (ctx) => { const styled = { added: false } const byKey = (key: string) => (key.startsWith("ssh:") ? ssh.item(key.slice(4)) : undefined) - const open = (module: Awaited>, render: () => JSX.Element) => { + // The dialog chunk under its load budget: one that fails or never loads rejects. + const boundedDialog = () => { + const deadline = Promise.withResolvers() + const timer = setTimeout(() => deadline.reject(new Error()), SSH_DIALOG_LOAD_TIMEOUT_MS) + + return Promise.race([loadDialog(), deadline.promise]).finally(() => clearTimeout(timer)) + } + + const open = (module: DialogModule, render: () => JSX.Element) => { if (!styled.added) ctx.add(Style, module.css) styled.added = true dialog.open(render) } - const show = (render: (module: Awaited>) => JSX.Element) => - void loadDialog().then((module) => { - if (ctx.signal.aborted) return - open(module, () => render(module)) - }) + const [addState, setAddState] = createSignal(initialSshAdd) - const add = (openProject: boolean) => - show((module) => ( + const dispatchAdd = (event: SshAddEvent) => { + const next = transitionSshAdd(untrack(addState), event) + + if (next.state !== untrack(addState)) setAddState(next.state) + next.effects.forEach(runAdd) + } + + const runAdd = (effect: SshAddEffect) => { + if (effect.type === "toast-failed") + return void showToast({ + variant: "error", + title: ctx.t("common.requestFailed"), + description: effect.error || undefined, + }) + + if (effect.type === "load") { + const attempt = effect.attempt + + return void boundedDialog().then( + (module) => dispatchAdd({ type: "add.loaded", attempt, module }), + (cause: unknown) => + dispatchAdd({ type: "add.failed", attempt, error: cause instanceof Error ? cause.message : String(cause) }), + ) + } + + const module = effect.module + + open(module, () => ( { const item = ssh.item(id) @@ -85,6 +124,11 @@ const setup: Plugin.Setup = (ctx) => { } /> )) + } + + onCleanup(() => dispatchAdd({ type: "disposed" })) + + const add = (openProject: boolean) => dispatchAdd({ type: "add.requested", openProject }) const entry = (item: SshItem): ServerEntry => { const id = item.config.id @@ -183,7 +227,8 @@ const setup: Plugin.Setup = (ctx) => { const id = target.item.config.id const config = target.item.config ssh.dialog.loading(id, target.attempt) - void loadDialog().then( + // A chunk that never loads ends the attempt too, so it never waits on a dialog that cannot show. + void boundedDialog().then( (module) => { if (ctx.signal.aborted || !ssh.dialog.canOpen(id, target.attempt)) return ssh.dialog.opened(id, target.attempt) diff --git a/packages/gui-plugins/src/terminal/focus-machine.test.ts b/packages/gui-plugins/src/terminal/focus-machine.test.ts index d6c1ad302b5..0956ba45a74 100644 --- a/packages/gui-plugins/src/terminal/focus-machine.test.ts +++ b/packages/gui-plugins/src/terminal/focus-machine.test.ts @@ -95,6 +95,35 @@ describe("terminal focus machine", () => { expect(focusRequestToken(transitionFocus(plain, target("pty_2")), "ses_1", "pty_2")).toBe(2) }) + // A second new-terminal request joins the create in flight; it used to replace the request, so the create's answer + // for the first one was stale and the new terminal took no focus. + test("asking again for a terminal being created keeps the request", () => { + const state = replay([pending()]) + + expect(transitionFocus(state, pending())).toBe(state) + expect(transitionFocus(state, pending(true))).toEqual({ + status: "pending", + request: 1, + session: "ses_1", + overlay: true, + }) + expect( + isFocusRequested( + replay([pending(), { type: "pending.resolved", request: 1, id: "pty_1" }], state), + "ses_1", + "pty_1", + ), + ).toBe(true) + + // Once the user moved on, a new request starts again. + expect(replay([{ type: "cancelled" }, pending()], state)).toEqual({ + status: "pending", + request: 2, + session: "ses_1", + overlay: false, + }) + }) + test("the terminal taking focus, or a cancel for its session, ends the request", () => { const state = replay([target("pty_1")]) diff --git a/packages/gui-plugins/src/terminal/focus-machine.ts b/packages/gui-plugins/src/terminal/focus-machine.ts index 09cdec66b65..224c0c39328 100644 --- a/packages/gui-plugins/src/terminal/focus-machine.ts +++ b/packages/gui-plugins/src/terminal/focus-machine.ts @@ -45,8 +45,13 @@ export const initialFocusState: FocusState = { status: "idle", request: 0 } * return of a closing dialog or menu that started the request does not. */ export function transitionFocus(state: FocusState, event: FocusRequestEvent): FocusState { - if (event.type === "request.pending") + if (event.type === "request.pending") { + // A second new-terminal request joins the create in flight, so it keeps that create's request. + if (state.status === "pending" && state.session === event.session) + return event.overlay && !state.overlay ? { ...state, overlay: true } : state + return { status: "pending", request: state.request + 1, session: event.session, overlay: event.overlay } + } if (event.type === "request.target") { // Showing the tab asks again for the terminal already asked for; that keeps the request and its overlay. diff --git a/packages/gui-plugins/src/terminal/i18n/en.ts b/packages/gui-plugins/src/terminal/i18n/en.ts index db498819638..5c568935e31 100644 --- a/packages/gui-plugins/src/terminal/i18n/en.ts +++ b/packages/gui-plugins/src/terminal/i18n/en.ts @@ -9,6 +9,8 @@ export default { unavailable: "Terminals aren't available on this server", "title.numbered": "Terminal {{number}}", close: "Close terminal", + "close.failed": "Couldn't close the terminal", + "request.timedOut": "The server took too long to answer", "create.failed": "Couldn't create a terminal", "create.unsupported": "This server is too old for terminals. Update OpenCode on this server.", "rename.failed": "Couldn't rename the terminal", diff --git a/packages/gui-plugins/src/terminal/machine.test.ts b/packages/gui-plugins/src/terminal/machine.test.ts index 47e4bf4adb8..26b799d097e 100644 --- a/packages/gui-plugins/src/terminal/machine.test.ts +++ b/packages/gui-plugins/src/terminal/machine.test.ts @@ -4,9 +4,11 @@ import { initialState, serverAvailable, sessionConnection, + sessionCreating, sessionView, tabHidden, transition, + viewItems, viewTabs, type Event, type State, @@ -100,7 +102,10 @@ describe("terminal machine", () => { { type: "server.connected" }, ]) - expect(rechecked.commands).toEqual([{ type: "check-capability", attempt: 2 }]) + expect(rechecked.commands).toEqual([ + { type: "abort", attempts: [1] }, + { type: "check-capability", attempt: 2 }, + ]) const stale = replay( [ @@ -183,7 +188,11 @@ describe("terminal machine", () => { const rechecking = transition(supported.state, { type: "server.connected" }) - expect(rechecking.commands).toEqual([{ type: "check-capability", attempt: 4 }]) + // The reconnect stops the list it no longer waits for. + expect(rechecking.commands).toEqual([ + { type: "abort", attempts: [3] }, + { type: "check-capability", attempt: 4 }, + ]) expect(sessionView(rechecking.state, "ses_1")).toEqual({ type: "ready", items: [info("pty_1")] }) expect(sessionView(rechecking.state, "ses_2")).toEqual({ type: "loading" }) @@ -216,51 +225,221 @@ describe("terminal machine", () => { expect(unknown.commands).toEqual([{ type: "load-list", sessionID: "ses_1", attempt: 3 }]) - // A local change to a terminal the list doesn't have, or an update for a session never loaded, changes nothing. - expect(transition(state, { type: "terminal.replaced", sessionID: "ses_1", terminal: info("pty_9") }).state).toBe( - state, - ) + // An update for a session never loaded changes nothing. expect(transition(state, { type: "terminal.updated", sessionID: "ses_9", terminal: info("pty_9") }).state).toBe( state, ) }) - // The create answer and the server's `persistent-pty.added` race: the creator's refresh can lose to the one the event - // started, which answers later. The new tab is listed meanwhile, so selecting it settles focus at once. + // The create answer and the server's `persistent-pty.added` race: the refresh the event started can answer without + // the new terminal. The new tab is listed at once, so selecting it settles focus. test("a terminal this client created is listed before a newer refresh answers", () => { - const own = transition(listed(), { type: "session.refresh", sessionID: "ses_1" }).state - const added = transition(own, { type: "session.refresh", sessionID: "ses_1" }).state - const lost = transition(added, { type: "list.loaded", sessionID: "ses_1", attempt: 3, items: [info("pty_1")] }) + const requested = transition(listed(), { type: "create.requested", sessionID: "ses_1" }) - expect(lost.state).toBe(added) + expect(requested.commands).toEqual([ + { type: "create-terminal", sessionID: "ses_1", attempt: 3, known: [info("pty_1")] }, + ]) - const created = transition(lost.state, { type: "terminal.created", sessionID: "ses_1", terminal: info("pty_2") }) + const added = transition(requested.state, { type: "session.refresh", sessionID: "ses_1" }).state - expect(created.commands).toEqual([]) + const created = transition(added, { + type: "create.succeeded", + sessionID: "ses_1", + attempt: 3, + terminal: info("pty_2"), + }) + + expect(created.commands).toEqual([ + { type: "settle-create", sessionID: "ses_1", outcome: { ok: true, terminal: info("pty_2") } }, + { type: "abort", attempts: [4] }, + { type: "load-list", sessionID: "ses_1", attempt: 5 }, + ]) expect(sessionView(created.state, "ses_1")).toEqual({ type: "ready", items: [info("pty_1"), info("pty_2")] }) + expect(sessionCreating(created.state, "ses_1")).toBe(false) - const settled = transition(created.state, { + // The refresh it replaced answers without the new terminal; it is stale. + const stale = transition(created.state, { type: "list.loaded", sessionID: "ses_1", attempt: 4, - items: [info("pty_1"), info("pty_2")], + items: [info("pty_1")], }) - expect(sessionView(settled.state, "ses_1")).toEqual({ type: "ready", items: [info("pty_1"), info("pty_2")] }) + expect(stale.state).toBe(created.state) + }) - // A terminal already listed, or a list not known yet, changes nothing. - expect( - transition(created.state, { type: "terminal.created", sessionID: "ses_1", terminal: info("pty_2") }).state, - ).toBe(created.state) + // Double-clicking "+" or pressing the shortcut twice used to send two creates, both titled "Terminal 1". + test("a second new-terminal request joins the create in flight", () => { + const first = transition(listed(), { type: "create.requested", sessionID: "ses_1" }) + const second = transition(first.state, { type: "create.requested", sessionID: "ses_1" }) - const loading = replay([ - { type: "session.loaded", sessionID: "ses_1" }, - { type: "capability.checked", attempt: 1, supported: true }, - ]).state + expect(first.commands).toEqual([ + { type: "create-terminal", sessionID: "ses_1", attempt: 3, known: [info("pty_1")] }, + ]) + expect(second.state).toBe(first.state) + expect(second.commands).toEqual([]) + expect(sessionCreating(first.state, "ses_1")).toBe(true) - expect(transition(loading, { type: "terminal.created", sessionID: "ses_1", terminal: info("pty_2") }).state).toBe( - loading, + // A failed or timed-out create answers every request it joined once, and the next request creates again. + const failed = transition(first.state, { + type: "create.failed", + sessionID: "ses_1", + attempt: 3, + kind: "failed", + error: "timed out", + }) + + expect(failed.commands).toEqual([ + { type: "settle-create", sessionID: "ses_1", outcome: { ok: false, kind: "failed", error: "timed out" } }, + ]) + expect(sessionCreating(failed.state, "ses_1")).toBe(false) + expect(transition(failed.state, { type: "create.requested", sessionID: "ses_1" }).commands).toEqual([ + { type: "create-terminal", sessionID: "ses_1", attempt: 4, known: [info("pty_1")] }, + ]) + }) + + test("a create while support is checked lists first, and a server without terminals answers it at once", () => { + const checking = replay([{ type: "server.connected" }, { type: "session.loaded", sessionID: "ses_1" }]).state + const requested = transition(checking, { type: "create.requested", sessionID: "ses_1" }) + + expect(requested.commands).toEqual([{ type: "create-terminal", sessionID: "ses_1", attempt: 2, known: undefined }]) + + // Losing support ends the create in flight and tells the user why. + const unsupported = transition(requested.state, { type: "capability.checked", attempt: 1, supported: false }) + + expect(unsupported.commands).toEqual([ + { type: "abort", attempts: [2] }, + { type: "settle-create", sessionID: "ses_1", outcome: { ok: false, kind: "unsupported" } }, + ]) + expect(transition(unsupported.state, { type: "create.requested", sessionID: "ses_1" }).commands).toEqual([ + { type: "settle-create", sessionID: "ses_1", outcome: { ok: false, kind: "unsupported" } }, + ]) + }) + + // An older rename that answered after a newer one used to put the older title back; two failed renames rolled back + // to the first one's unconfirmed title. + test("only the newest rename settles a terminal's title", () => { + const alpha = transition(listed(), { type: "rename.requested", sessionID: "ses_1", ptyID: "pty_1", title: "alpha" }) + + const beta = transition(alpha.state, { + type: "rename.requested", + sessionID: "ses_1", + ptyID: "pty_1", + title: "beta", + }) + + expect(beta.commands).toEqual([ + { type: "abort", attempts: [3] }, + { type: "rename-terminal", sessionID: "ses_1", ptyID: "pty_1", title: "beta", attempt: 4 }, + ]) + expect(viewItems(sessionView(beta.state, "ses_1"))[0]).toMatchObject({ title: "beta", userTitle: "beta" }) + + const renamed = (title: string) => ({ ...info("pty_1", title), userTitle: title }) + + const settled = replay( + [ + { type: "rename.succeeded", sessionID: "ses_1", ptyID: "pty_1", attempt: 4, terminal: renamed("beta") }, + { type: "rename.succeeded", sessionID: "ses_1", ptyID: "pty_1", attempt: 3, terminal: renamed("alpha") }, + ], + beta.state, ) + + expect(viewItems(sessionView(settled.state, "ses_1"))).toEqual([renamed("beta")]) + + const failed = replay( + [ + { type: "rename.failed", sessionID: "ses_1", ptyID: "pty_1", attempt: 3, kind: "failed", error: "old" }, + { type: "rename.failed", sessionID: "ses_1", ptyID: "pty_1", attempt: 4, kind: "failed", error: "boom" }, + ], + beta.state, + ) + + // The confirmed title shows again, not the first rename's. + expect(viewItems(sessionView(failed.state, "ses_1"))).toEqual([info("pty_1")]) + expect(failed.commands).toEqual([{ type: "toast-rename-failed", kind: "failed", error: "boom" }]) + + // Asking for the title a pending rename already sends, or the current title, sends nothing. + expect( + transition(beta.state, { type: "rename.requested", sessionID: "ses_1", ptyID: "pty_1", title: "beta" }).state, + ).toBe(beta.state) + expect( + transition(listed(), { type: "rename.requested", sessionID: "ses_1", ptyID: "pty_1", title: "pty_1" }).commands, + ).toEqual([]) + }) + + // A failed close used to end in a console line: the tab stayed gone with its terminal still running. + test("a close runs once, ends a pending rename, and brings the tab back when it fails", () => { + const renaming = transition(listed(), { + type: "rename.requested", + sessionID: "ses_1", + ptyID: "pty_1", + title: "alpha", + }).state + + const closing = transition(renaming, { type: "close.requested", sessionID: "ses_1", ptyID: "pty_1" }) + + expect(closing.commands).toEqual([ + { type: "abort", attempts: [3] }, + { type: "close-terminal", sessionID: "ses_1", ptyID: "pty_1", attempt: 4 }, + ]) + expect(transition(closing.state, { type: "close.requested", sessionID: "ses_1", ptyID: "pty_1" }).state).toBe( + closing.state, + ) + expect( + transition(closing.state, { type: "rename.requested", sessionID: "ses_1", ptyID: "pty_1", title: "beta" }).state, + ).toBe(closing.state) + + const failed = transition(closing.state, { + type: "close.failed", + sessionID: "ses_1", + ptyID: "pty_1", + attempt: 4, + error: "boom", + }) + + expect(failed.commands).toEqual([ + { type: "sync-tabs", sessionID: "ses_1", items: [info("pty_1")] }, + { type: "toast-close-failed", error: "boom" }, + ]) + + const closed = transition(closing.state, { + type: "close.succeeded", + sessionID: "ses_1", + ptyID: "pty_1", + attempt: 4, + }) + + expect(viewItems(sessionView(closed.state, "ses_1"))).toEqual([]) + expect(closed.commands).toEqual([{ type: "load-list", sessionID: "ses_1", attempt: 5 }]) + }) + + test("removing a session or stopping the model ends its changes in flight and keeps confirmed lists", () => { + const busy = replay( + [ + { type: "create.requested", sessionID: "ses_1" }, + { type: "rename.requested", sessionID: "ses_1", ptyID: "pty_1", title: "alpha" }, + { type: "session.refresh", sessionID: "ses_1" }, + ], + listed(), + ).state + + const removed = transition(busy, { type: "sessions.removed", sessionIDs: ["ses_1"] }) + + expect(removed.commands).toEqual([ + { type: "abort", attempts: [5, 3, 4] }, + { type: "settle-create", sessionID: "ses_1", outcome: { ok: false, kind: "cancelled" } }, + ]) + expect(sessionCreating(removed.state, "ses_1")).toBe(false) + + const stopped = transition(busy, { type: "instance.stopped" }) + + expect(stopped.commands).toEqual([ + { type: "settle-create", sessionID: "ses_1", outcome: { ok: false, kind: "cancelled" } }, + ]) + expect(sessionCreating(stopped.state, "ses_1")).toBe(false) + // The pending rename's title leaves with it; the confirmed list stays for the next instance. + expect(viewItems(sessionView(stopped.state, "ses_1"))).toEqual([info("pty_1")]) + expect(transition(stopped.state, { type: "instance.stopped" }).state).toBe(stopped.state) }) test("answers for a removed session are ignored", () => { diff --git a/packages/gui-plugins/src/terminal/machine.ts b/packages/gui-plugins/src/terminal/machine.ts index 96166edb3ce..050a151dfa2 100644 --- a/packages/gui-plugins/src/terminal/machine.ts +++ b/packages/gui-plugins/src/terminal/machine.ts @@ -1,8 +1,14 @@ import type { PersistentPtyInfo } from "@opencode/client" +/** + * How long one terminal request may take before it fails: the support check, a list, a create (including the wait + * for a session the server is still creating), a close or a rename. A server can accept a request and never answer. + */ +export const TERMINAL_REQUEST_TIMEOUT_MS = 15_000 + /** * A session's terminal list once the server offers terminals. `connection` is the server connection whose answer the - * known list is: a refresh keeps the previous one until it answers. + * known list is: a refresh keeps the previous one until it answers. `items` holds only what the server confirmed. */ export type List = | { readonly status: "loading"; readonly attempt: number } @@ -20,6 +26,17 @@ export type Held = | { readonly status: "waiting" } | { readonly status: "ready"; readonly connection: number; readonly items: readonly PersistentPtyInfo[] } +/** + * This client's terminal changes in flight for one session, each with its request's attempt. Every new-terminal + * request joins the one `creating` create. A terminal has at most one `closing` request and one rename; a pending + * rename's title shows over the confirmed list until the server answers. + */ +export type Mutations = { + readonly creating?: number + readonly closing: Readonly> + readonly renames: Readonly> +} + /** * One server's terminal support and the terminal lists of its loaded sessions. Each request takes the server's next * attempt number, so an answer whose attempt the state no longer holds is stale. `issued` is the last one handed out. @@ -31,7 +48,9 @@ export type State = readonly status: "checking" readonly connections: number readonly attempt: number + readonly issued: number readonly sessions: Readonly> + readonly mutations: Readonly> } | { readonly status: "unsupported" @@ -51,12 +70,17 @@ export type State = readonly connections: number readonly issued: number readonly sessions: Readonly> + readonly mutations: Readonly> } +/** Why a create ended without a terminal: an outdated server, a failed or timed-out request, or a cancel. */ +export type CreateFailure = "unsupported" | "failed" | "cancelled" + export type Event = /** The model started following the server, its event stream connected again, or a restarted server replaced it. */ | { readonly type: "server.connected" } | { readonly type: "capability.checked"; readonly attempt: number; readonly supported: boolean } + /** The support check failed or timed out. */ | { readonly type: "capability.failed"; readonly attempt: number; readonly error: string } | { readonly type: "session.loaded"; readonly sessionID: string } /** The server reported a terminal change, or this client changed the session's terminals. */ @@ -71,23 +95,100 @@ export type Event = } /** The list answered 503: the server's terminal daemon can't run. */ | { readonly type: "list.unavailable"; readonly sessionID: string; readonly attempt: number } + /** The list failed or timed out. */ | { readonly type: "list.failed"; readonly sessionID: string; readonly attempt: number; readonly error: string } /** The server sent a terminal's new info. */ | { readonly type: "terminal.updated"; readonly sessionID: string; readonly terminal: PersistentPtyInfo } - /** This client changed a listed terminal's info. */ - | { readonly type: "terminal.replaced"; readonly sessionID: string; readonly terminal: PersistentPtyInfo } - /** - * This client created a terminal. It joins a known list at once: the refresh that follows can lose to a newer one - * the server's `persistent-pty.added` started, and the tab must be listed when it is selected. - */ - | { readonly type: "terminal.created"; readonly sessionID: string; readonly terminal: PersistentPtyInfo } + /** The user asks for a new terminal. A request while one is being created joins it. */ + | { readonly type: "create.requested"; readonly sessionID: string } + | { + readonly type: "create.succeeded" + readonly sessionID: string + readonly attempt: number + readonly terminal: PersistentPtyInfo + } + /** The create failed or timed out. */ + | { + readonly type: "create.failed" + readonly sessionID: string + readonly attempt: number + readonly kind: "unsupported" | "failed" + readonly error: string + } + /** The user closes a terminal. A request while it is closing joins it. */ + | { readonly type: "close.requested"; readonly sessionID: string; readonly ptyID: string } + /** The server removed the terminal, or it was already gone. */ + | { readonly type: "close.succeeded"; readonly sessionID: string; readonly ptyID: string; readonly attempt: number } + /** The close failed or timed out. */ + | { + readonly type: "close.failed" + readonly sessionID: string + readonly ptyID: string + readonly attempt: number + readonly error: string + } + /** The user sets a terminal's title; `null` clears it. */ + | { + readonly type: "rename.requested" + readonly sessionID: string + readonly ptyID: string + readonly title: string | null + } + | { + readonly type: "rename.succeeded" + readonly sessionID: string + readonly ptyID: string + readonly attempt: number + readonly terminal: PersistentPtyInfo + } + /** The rename failed or timed out. */ + | { + readonly type: "rename.failed" + readonly sessionID: string + readonly ptyID: string + readonly attempt: number + readonly kind: "unsupported" | "failed" + readonly error: string + } | { readonly type: "sessions.removed"; readonly sessionIDs: readonly string[] } + /** + * The model goes away, as on a plugin reload. Changes in flight end; the confirmed lists stay for the next instance, + * which checks the server again. + */ + | { readonly type: "instance.stopped" } export type Command = | { readonly type: "check-capability"; readonly attempt: number } | { readonly type: "load-list"; readonly sessionID: string; readonly attempt: number } + /** Waits for a session the server is still creating, lists it when `known` is undefined, and creates a terminal. */ + | { + readonly type: "create-terminal" + readonly sessionID: string + readonly attempt: number + readonly known: readonly PersistentPtyInfo[] | undefined + } + | { readonly type: "close-terminal"; readonly sessionID: string; readonly ptyID: string; readonly attempt: number } + | { + readonly type: "rename-terminal" + readonly sessionID: string + readonly ptyID: string + readonly title: string | null + readonly attempt: number + } + /** Cancels requests whose answers the state no longer waits for. */ + | { readonly type: "abort"; readonly attempts: readonly number[] } /** Adds the listed terminals the strip doesn't store yet, such as ones another client created. */ | { readonly type: "sync-tabs"; readonly sessionID: string; readonly items: readonly PersistentPtyInfo[] } + /** Answers every new-terminal request the create joined: selects the terminal, or tells the user why not. */ + | { + readonly type: "settle-create" + readonly sessionID: string + readonly outcome: + | { readonly ok: true; readonly terminal: PersistentPtyInfo } + | { readonly ok: false; readonly kind: CreateFailure; readonly error?: string } + } + | { readonly type: "toast-close-failed"; readonly error: string } + | { readonly type: "toast-rename-failed"; readonly kind: "unsupported" | "failed"; readonly error: string } export type Transition = { readonly state: State; readonly commands: readonly Command[] } @@ -100,6 +201,8 @@ export type View = type Supported = Extract +type Active = Extract + export const initialState: State = { status: "unknown" } const LOADING: View = { type: "loading" } @@ -111,9 +214,17 @@ const NONE: readonly PersistentPtyInfo[] = [] export function transition(state: State, event: Event): Transition { if (state.status === "unknown") return unknown(state, event) - if (state.status === "checking") return checking(state, event) + if (state.status === "checking" || state.status === "supported") { + if (event.type === "instance.stopped") return stopped(state) - if (state.status === "supported") return supported(state, event) + if (event.type === "sessions.removed") return removed(state, event.sessionIDs) + + const mutated = mutate(state, event) + + if (mutated) return mutated + + return state.status === "checking" ? checking(state, event) : supported(state, event) + } return resting(state, event) } @@ -136,7 +247,14 @@ export function sessionView(state: State, sessionID: string): View { if (session.status === "failed") return { type: "failed", error: session.error } - return { type: "ready", items: session.items } + return { type: "ready", items: renamed(session.items, state.mutations[sessionID]) } +} + +/** A new terminal is being created for the session; another request joins it. */ +export function sessionCreating(state: State, sessionID: string) { + return ( + (state.status === "checking" || state.status === "supported") && active(state, sessionID).creating !== undefined + ) } /** @@ -177,24 +295,26 @@ export function tabTargetable(current: View, id: string) { } function unknown(state: State, event: Event): Transition { - if (event.type === "server.connected") return check(1, {}, 1) + if (event.type === "server.connected") return check(1, {}, 1, {}) - if (event.type === "session.loaded") return check(1, { [event.sessionID]: { status: "waiting" } }, 1) + if (event.type === "session.loaded") return check(1, { [event.sessionID]: { status: "waiting" } }, 1, {}) + + if (event.type === "create.requested") + return unchanged(state, [ + { type: "settle-create", sessionID: event.sessionID, outcome: { ok: false, kind: "cancelled" } }, + ]) return unchanged(state) } function checking(state: Extract, event: Event): Transition { - if (event.type === "server.connected") return check(state.attempt + 1, state.sessions, state.connections + 1) + if (event.type === "server.connected") + return check(state.issued + 1, state.sessions, state.connections + 1, state.mutations, [ + { type: "abort", attempts: [state.attempt] }, + ]) if (event.type === "capability.checked" && event.attempt === state.attempt) { - if (!event.supported) - return changed({ - status: "unsupported", - connections: state.connections, - issued: state.attempt, - sessions: Object.keys(state.sessions), - }) + if (!event.supported) return lost(state, { status: "unsupported" }) // Every session lists again; a list it already had stays shown meanwhile. const ids = Object.keys(state.sessions) @@ -202,7 +322,7 @@ function checking(state: Extract, event: Event): const sessions = Object.fromEntries( ids.map((id, index): [string, List] => { const held = state.sessions[id] - const attempt = state.attempt + 1 + index + const attempt = state.issued + 1 + index if (held?.status === "ready") return [id, { status: "refreshing", connection: held.connection, items: held.items, attempt }] @@ -212,39 +332,25 @@ function checking(state: Extract, event: Event): ) return changed( - { status: "supported", connections: state.connections, issued: state.attempt + ids.length, sessions }, - ids.map((sessionID, index): Command => ({ type: "load-list", sessionID, attempt: state.attempt + 1 + index })), + { + status: "supported", + connections: state.connections, + issued: state.issued + ids.length, + sessions, + mutations: state.mutations, + }, + ids.map((sessionID, index): Command => ({ type: "load-list", sessionID, attempt: state.issued + 1 + index })), ) } if (event.type === "capability.failed" && event.attempt === state.attempt) - return changed({ - status: "failed", - connections: state.connections, - issued: state.attempt, - sessions: Object.keys(state.sessions), - error: event.error, - }) + return lost(state, { status: "failed", error: event.error }) if (event.type === "session.loaded" && !state.sessions[event.sessionID]) return changed({ ...state, sessions: { ...state.sessions, [event.sessionID]: { status: "waiting" } } }) - if (event.type === "terminal.updated" || event.type === "terminal.replaced") { - const held = state.sessions[event.sessionID] - - if (held?.status !== "ready" || !listed(held.items, event.terminal.id)) return unchanged(state) - - return changed({ - ...state, - sessions: { - ...state.sessions, - [event.sessionID]: { ...held, items: replace(held.items, event.terminal) }, - }, - }) - } - - if (event.type === "sessions.removed" && event.sessionIDs.some((id) => id in state.sessions)) - return changed({ ...state, sessions: omit(state.sessions, event.sessionIDs) }) + if (event.type === "terminal.updated") + return changed(withItems(state, event.sessionID, (items) => replace(items, event.terminal))) return unchanged(state) } @@ -255,6 +361,7 @@ function resting(state: Extract, ev state.issued + 1, Object.fromEntries(state.sessions.map((id): [string, Held] => [id, { status: "waiting" }])), event.type === "server.connected" ? state.connections + 1 : state.connections, + {}, ) if (event.type === "session.loaded" && !state.sessions.includes(event.sessionID)) @@ -263,6 +370,19 @@ function resting(state: Extract, ev if (event.type === "sessions.removed" && state.sessions.some((id) => event.sessionIDs.includes(id))) return changed({ ...state, sessions: state.sessions.filter((id) => !event.sessionIDs.includes(id)) }) + // A server without terminals answers a new-terminal request at once. + if (event.type === "create.requested") + return unchanged(state, [ + { + type: "settle-create", + sessionID: event.sessionID, + outcome: + state.status === "unsupported" + ? { ok: false, kind: "unsupported" } + : { ok: false, kind: "failed", error: state.error }, + }, + ]) + return unchanged(state) } @@ -279,6 +399,8 @@ function supported(state: Supported, event: Event): Transition { }), ), state.connections + 1, + state.mutations, + abort(Object.values(state.sessions).flatMap(listAttempt)), ) const list = "sessionID" in event ? state.sessions[event.sessionID] : undefined @@ -302,13 +424,7 @@ function supported(state: Supported, event: Event): Transition { ) // A server whose terminal daemon can't run offers no terminals until the next connection checks again. - if (event.type === "list.unavailable") - return changed({ - status: "unsupported", - connections: state.connections, - issued: state.issued, - sessions: Object.keys(state.sessions), - }) + if (event.type === "list.unavailable") return lost(state, { status: "unsupported" }) // A failed refresh keeps the terminals already shown. if (list.status === "refreshing") @@ -317,23 +433,222 @@ function supported(state: Supported, event: Event): Transition { return changed(put(state, event.sessionID, { status: "failed", error: event.error })) } - if (event.type === "terminal.updated" || event.type === "terminal.replaced") { + if (event.type === "terminal.updated") { if (!list) return unchanged(state) if ((list.status === "ready" || list.status === "refreshing") && listed(list.items, event.terminal.id)) return changed(put(state, event.sessionID, { ...list, items: replace(list.items, event.terminal) })) - // A terminal the list doesn't have yet arrives with the next list; this client changes only listed ones. - return event.type === "terminal.updated" ? load(state, event.sessionID) : unchanged(state) + // A terminal the list doesn't have yet arrives with the next list. + return load(state, event.sessionID) } - if (event.type === "terminal.created") - return (list?.status === "ready" || list?.status === "refreshing") && !listed(list.items, event.terminal.id) - ? changed(put(state, event.sessionID, { ...list, items: [...list.items, event.terminal] })) - : unchanged(state) + return unchanged(state) +} - if (event.type === "sessions.removed" && event.sessionIDs.some((id) => id in state.sessions)) - return changed({ ...state, sessions: omit(state.sessions, event.sessionIDs) }) +/** This client's creates, closes and renames, the same while support is checked again and once it is known. */ +function mutate(state: Active, event: Event): Transition | undefined { + if (!("sessionID" in event)) return + const sessionID = event.sessionID + const current = active(state, sessionID) + + if (event.type === "create.requested") { + if (current.creating !== undefined) return unchanged(state) + const attempt = state.issued + 1 + + return changed(withMutations({ ...state, issued: attempt }, sessionID, { ...current, creating: attempt }), [ + { type: "create-terminal", sessionID, attempt, known: known(state, sessionID) }, + ]) + } + + if (event.type === "create.succeeded") { + // A create that the state stopped waiting for still made a terminal; the list shows it once it refreshes. + if (current.creating !== event.attempt) return refresh(state, sessionID) + + const next = withItems( + withMutations(state, sessionID, { closing: current.closing, renames: current.renames }), + sessionID, + (items) => (listed(items, event.terminal.id) ? items : [...items, event.terminal]), + ) + + const settle: Command = { type: "settle-create", sessionID, outcome: { ok: true, terminal: event.terminal } } + const refreshed = refresh(next, sessionID) + + return changed(refreshed.state, [settle, ...refreshed.commands]) + } + + if (event.type === "create.failed") { + if (current.creating !== event.attempt) return unchanged(state) + + return changed(withMutations(state, sessionID, { closing: current.closing, renames: current.renames }), [ + { type: "settle-create", sessionID, outcome: { ok: false, kind: event.kind, error: event.error } }, + ]) + } + + if (event.type === "close.requested") { + if (current.closing[event.ptyID] !== undefined) return unchanged(state) + const attempt = state.issued + 1 + // Closing a terminal ends its rename. + const rename = current.renames[event.ptyID] + + return changed( + withMutations({ ...state, issued: attempt }, sessionID, { + ...current, + closing: { ...current.closing, [event.ptyID]: attempt }, + renames: without(current.renames, event.ptyID), + }), + [...abort(rename ? [rename.attempt] : []), { type: "close-terminal", sessionID, ptyID: event.ptyID, attempt }], + ) + } + + if (event.type === "close.succeeded") { + if (current.closing[event.ptyID] !== event.attempt) return unchanged(state) + + return refresh( + withItems( + withMutations(state, sessionID, { ...current, closing: without(current.closing, event.ptyID) }), + sessionID, + (items) => items.filter((info) => info.id !== event.ptyID), + ), + sessionID, + ) + } + + if (event.type === "close.failed") { + if (current.closing[event.ptyID] !== event.attempt) return unchanged(state) + const items = known(state, sessionID) + + // The terminal still runs, so its tab, which the strip already let go of, comes back. + return changed(withMutations(state, sessionID, { ...current, closing: without(current.closing, event.ptyID) }), [ + ...(items ? [{ type: "sync-tabs", sessionID, items } satisfies Command] : []), + { type: "toast-close-failed", error: event.error }, + ]) + } + + if (event.type === "rename.requested") { + const item = known(state, sessionID)?.find((info) => info.id === event.ptyID) + const pending = current.renames[event.ptyID] + + if (!item || current.closing[event.ptyID] !== undefined) return unchanged(state) + + // The title already shown, or clearing a terminal never renamed, sends nothing. + if ( + pending + ? pending.title === event.title + : event.title === item.title || (event.title === null && item.userTitle === null) + ) + return unchanged(state) + const attempt = state.issued + 1 + + return changed( + withMutations({ ...state, issued: attempt }, sessionID, { + ...current, + renames: { ...current.renames, [event.ptyID]: { attempt, title: event.title } }, + }), + [ + ...abort(pending ? [pending.attempt] : []), + { type: "rename-terminal", sessionID, ptyID: event.ptyID, title: event.title, attempt }, + ], + ) + } + + if (event.type === "rename.succeeded" || event.type === "rename.failed") { + // An older rename's answer is stale: the newer one decides, and the server's update event reports the result. + if (current.renames[event.ptyID]?.attempt !== event.attempt) return unchanged(state) + const next = withMutations(state, sessionID, { ...current, renames: without(current.renames, event.ptyID) }) + + if (event.type === "rename.failed") + return changed(next, [{ type: "toast-rename-failed", kind: event.kind, error: event.error }]) + + return changed(withItems(next, sessionID, (items) => replace(items, event.terminal))) + } + + return +} + +/** Support went away: every change in flight ends, and new-terminal requests learn why. */ +function lost( + state: Active, + next: { readonly status: "unsupported" } | { readonly status: "failed"; readonly error: string }, +): Transition { + const base = { connections: state.connections, issued: state.issued, sessions: Object.keys(state.sessions) } + + const outcome = + next.status === "unsupported" + ? ({ ok: false, kind: "unsupported" } as const) + : ({ ok: false, kind: "failed", error: next.error } as const) + + return changed( + next.status === "unsupported" + ? { status: "unsupported", ...base } + : { status: "failed", ...base, error: next.error }, + [ + ...abort(attempts(state, [...new Set([...Object.keys(state.sessions), ...Object.keys(state.mutations)])])), + ...settleCreates(state, Object.keys(state.mutations), outcome), + ], + ) +} + +function removed(state: Active, ids: readonly string[]): Transition { + if (!ids.some((id) => id in state.sessions || id in state.mutations)) return unchanged(state) + const mutations = omit(state.mutations, ids) + + const commands = [ + ...abort(attempts(state, ids)), + ...settleCreates(state, ids, { ok: false, kind: "cancelled" } as const), + ] + + if (state.status === "checking") + return changed({ ...state, sessions: omit(state.sessions, ids), mutations }, commands) + + return changed({ ...state, sessions: omit(state.sessions, ids), mutations }, commands) +} + +/** The confirmed lists stay for the next instance; the changes in flight end with this one. */ +function stopped(state: Active): Transition { + if (Object.keys(state.mutations).length === 0) return unchanged(state) + + return changed( + { ...state, mutations: {} }, + settleCreates(state, Object.keys(state.mutations), { ok: false, kind: "cancelled" } as const), + ) +} + +function settleCreates( + state: Active, + ids: readonly string[], + outcome: { readonly ok: false; readonly kind: CreateFailure; readonly error?: string }, +): Command[] { + return ids.flatMap((sessionID): Command[] => + state.mutations[sessionID]?.creating === undefined ? [] : [{ type: "settle-create", sessionID, outcome }], + ) +} + +/** Every request in flight for the sessions: their lists and this client's changes. */ +function attempts(state: Active, ids: readonly string[]) { + return ids.flatMap((id) => { + const entry = state.sessions[id] + const mutations = state.mutations[id] + const list = entry && entry.status !== "waiting" ? listAttempt(entry) : [] + + if (!mutations) return list + + return [ + ...list, + ...(mutations.creating === undefined ? [] : [mutations.creating]), + ...Object.values(mutations.closing), + ...Object.values(mutations.renames).map((rename) => rename.attempt), + ] + }) +} + +function listAttempt(list: List) { + return list.status === "loading" || list.status === "refreshing" ? [list.attempt] : [] +} + +/** Lists the session again once support is known; while it is checked, the check's lists will. */ +function refresh(state: Active, sessionID: string): Transition { + if (state.status === "supported" && state.sessions[sessionID]) return load(state, sessionID) return unchanged(state) } @@ -347,19 +662,92 @@ function load(state: Supported, sessionID: string): Transition { ? ({ status: "refreshing", connection: list.connection, items: list.items, attempt } satisfies List) : ({ status: "loading", attempt } satisfies List) - return changed({ ...put(state, sessionID, next), issued: attempt }, [{ type: "load-list", sessionID, attempt }]) + // Only the newest list may answer, so the one it replaces stops. + return changed({ ...put(state, sessionID, next), issued: attempt }, [ + ...abort(list ? listAttempt(list) : []), + { type: "load-list", sessionID, attempt }, + ]) } -function check(attempt: number, sessions: Readonly>, connections: number): Transition { - return changed({ status: "checking", connections, attempt, sessions }, [{ type: "check-capability", attempt }]) +function check( + attempt: number, + sessions: Readonly>, + connections: number, + mutations: Readonly>, + commands: readonly Command[] = [], +): Transition { + return changed({ status: "checking", connections, attempt, issued: attempt, sessions, mutations }, [ + ...commands, + { type: "check-capability", attempt }, + ]) +} + +function abort(attempts: readonly number[]): Command[] { + return attempts.length > 0 ? [{ type: "abort", attempts }] : [] +} + +/** The session's changes in flight; a fresh object each time, as the state lives in a store. */ +function active(state: Active, sessionID: string): Mutations { + return state.mutations[sessionID] ?? { closing: {}, renames: {} } +} + +function known(state: Active, sessionID: string) { + const entry = state.sessions[sessionID] + + return entry?.status === "ready" || entry?.status === "refreshing" ? entry.items : undefined +} + +function withMutations(state: T, sessionID: string, next: Mutations): T { + const idle = + next.creating === undefined && Object.keys(next.closing).length === 0 && Object.keys(next.renames).length === 0 + + return { ...state, mutations: idle ? omit(state.mutations, [sessionID]) : { ...state.mutations, [sessionID]: next } } +} + +/** Changes a known list's confirmed terminals; a list not known yet stays as it is. */ +function withItems( + state: T, + sessionID: string, + change: (items: readonly PersistentPtyInfo[]) => readonly PersistentPtyInfo[], +): T { + if (state.status === "checking") { + const held = state.sessions[sessionID] + + if (held?.status !== "ready") return state + const items = change(held.items) + + return items === held.items ? state : { ...state, sessions: { ...state.sessions, [sessionID]: { ...held, items } } } + } + + const list = state.sessions[sessionID] + + if (list?.status !== "ready" && list?.status !== "refreshing") return state + const items = change(list.items) + + return items === list.items ? state : { ...state, sessions: { ...state.sessions, [sessionID]: { ...list, items } } } +} + +/** Shows each pending rename's new title over the confirmed list; a cleared title shows what the server falls back to. */ +function renamed(items: readonly PersistentPtyInfo[], mutations: Mutations | undefined) { + if (!mutations || !Object.values(mutations.renames).some((rename) => rename.title !== null)) return items + + return items.map((info) => { + const title = mutations.renames[info.id]?.title + + return title === undefined || title === null ? info : { ...info, title, userTitle: title } + }) } function put(state: Supported, sessionID: string, list: List): Supported { return { ...state, sessions: { ...state.sessions, [sessionID]: list } } } -function omit(sessions: Readonly>, ids: readonly string[]) { - return Object.fromEntries(Object.entries(sessions).filter(([id]) => !ids.includes(id))) +function omit(record: Readonly>, ids: readonly string[]) { + return Object.fromEntries(Object.entries(record).filter(([id]) => !ids.includes(id))) +} + +function without(record: Readonly>, id: string) { + return omit(record, [id]) } function listed(list: readonly PersistentPtyInfo[], id: string) { @@ -367,13 +755,13 @@ function listed(list: readonly PersistentPtyInfo[], id: string) { } function replace(list: readonly PersistentPtyInfo[], terminal: PersistentPtyInfo) { - return list.map((info) => (info.id === terminal.id ? terminal : info)) + return listed(list, terminal.id) ? list.map((info) => (info.id === terminal.id ? terminal : info)) : list } function changed(state: State, commands: readonly Command[] = []): Transition { return { state, commands } } -function unchanged(state: State): Transition { - return { state, commands: [] } +function unchanged(state: State, commands: readonly Command[] = []): Transition { + return { state, commands } } diff --git a/packages/gui-plugins/src/terminal/model.test.ts b/packages/gui-plugins/src/terminal/model.test.ts index 287c8b3f18c..2ee7086a463 100644 --- a/packages/gui-plugins/src/terminal/model.test.ts +++ b/packages/gui-plugins/src/terminal/model.test.ts @@ -1,12 +1,18 @@ -import { describe, expect, test } from "bun:test" +import { describe, expect, jest, test } from "bun:test" import { createStore, produce } from "solid-js/store" import { ClientError, type PersistentPtyInfo } from "@opencode/client" import type { Layout, ServerRef, SessionRef, Storage, ToastOptions } from "@opencode/plugin/gui" -import { PtyFeatureUnsupportedError, ServiceUnavailableError } from "@opencode/protocol/errors" +import { PtyFeatureUnsupportedError, PtyNotFoundError, ServiceUnavailableError } from "@opencode/protocol/errors" +import { TERMINAL_REQUEST_TIMEOUT_MS } from "./machine" import { createTerminalModel } from "./model" const flush = () => new Promise((resolve) => setTimeout(resolve, 0)) +// Settles pending promise callbacks while fake timers hold `flush`. +const ticks = async () => { + for (const _ of Array.from({ length: 20 })) await Promise.resolve() +} + const info = (id: string, title: string, sessionID = "ses_1"): PersistentPtyInfo => ({ id, title, @@ -74,6 +80,7 @@ function fakeServer(options: { capabilities?: { persistentPty?: boolean } } = {} }, create: async (input: { sessionID: string; title: string; cwd?: string }) => { calls.create.push(input) + await state.create if (state.createError) throw state.createError @@ -85,6 +92,8 @@ function fakeServer(options: { capabilities?: { persistentPty?: boolean } } = {} }, remove: async (input: { ptyID: string }) => { calls.remove.push(input.ptyID) + + if (state.removeError) throw state.removeError terminals.forEach((list, sessionID) => terminals.set( sessionID, @@ -132,6 +141,8 @@ type FakeState = { directory: string created: number createError?: Error + removeError?: Error + create?: Promise listError?: Error updateError?: Error update?: Promise @@ -337,7 +348,8 @@ describe("terminal model", () => { expect(server.calls.list).toBe(calls) server.state.listError = undefined - await terminal.retry() + terminal.retry() + await flush() expect(terminal.view().type).toBe("ready") expect(terminal.all().map((item) => item.id)).toEqual(["pty_1"]) @@ -431,7 +443,8 @@ describe("terminal model", () => { await flush() terminal.select("pty_1") - await terminal.close("pty_1") + terminal.close("pty_1") + await flush() expect(server.calls.remove).toEqual(["pty_1"]) expect(terminal.all().map((item) => item.id)).toEqual(["pty_2"]) @@ -471,12 +484,12 @@ describe("terminal model", () => { const terminal = model.load(session) await flush() - const renamed = terminal.rename("pty_1", "build") + terminal.rename("pty_1", "build") expect(terminal.get("pty_1")?.title).toBe("build") update.resolve() - await renamed + await flush() expect(server.calls.update).toEqual([{ ptyID: "pty_1", title: "build" }]) expect(terminal.get("pty_1")).toMatchObject({ title: "build", userTitle: "build" }) @@ -489,8 +502,9 @@ describe("terminal model", () => { const terminal = model.load(session) await flush() - await terminal.rename("pty_1", "Terminal 1") - await terminal.rename("pty_1", null) + terminal.rename("pty_1", "Terminal 1") + terminal.rename("pty_1", null) + await flush() expect(server.calls.update).toEqual([]) }) @@ -502,7 +516,8 @@ describe("terminal model", () => { const terminal = model.load(session) await flush() - await terminal.rename("pty_1", null) + terminal.rename("pty_1", null) + await flush() expect(server.calls.update).toEqual([{ ptyID: "pty_1", title: null }]) expect(terminal.get("pty_1")).toMatchObject({ title: "zsh", userTitle: null }) @@ -516,7 +531,8 @@ describe("terminal model", () => { const terminal = model.load(session) await flush() - await terminal.rename("pty_1", "build") + terminal.rename("pty_1", "build") + await flush() expect(toasts).toEqual([{ variant: "error", title: "rename.unsupported" }]) expect(terminal.get("pty_1")).toMatchObject({ title: "Terminal 1", userTitle: null }) @@ -530,7 +546,8 @@ describe("terminal model", () => { const terminal = model.load(session) await flush() - await terminal.rename("pty_1", "build") + terminal.rename("pty_1", "build") + await flush() expect(toasts).toEqual([{ variant: "error", title: "rename.failed", description: "boom" }]) expect(terminal.get("pty_1")?.title).toBe("Terminal 1") @@ -549,4 +566,119 @@ describe("terminal model", () => { }, ]) }) + + // Double-clicking "+" or pressing the shortcut twice used to create two terminals, both titled "Terminal 1". + test("a second new-terminal request joins the create in flight", async () => { + const server = fakeServer() + const create = Promise.withResolvers() + server.state.create = create.promise + const { model, session } = setup(server) + const terminal = model.load(session) + await flush() + + const first = terminal.new() + const second = terminal.new() + + expect(terminal.creating()).toBe(true) + create.resolve() + + expect(await Promise.all([first, second])).toEqual(["pty_new1", "pty_new1"]) + expect(server.calls.create).toEqual([{ sessionID: "ses_1", args: [], cwd: "/repo", title: "Terminal 1", env: {} }]) + expect(terminal.creating()).toBe(false) + expect(terminal.focusRequested("pty_new1")).toBe(true) + }) + + test("a terminal for a session the server is still creating waits for it, and a removal ends the wait", async () => { + const server = fakeServer() + const { model, session, toasts } = setup(server) + + const pending: SessionRef = { + ...session, + get pending() { + return true + }, + } + + const terminal = model.load(pending) + await flush() + + const created = terminal.new() + await flush() + + expect(server.calls.create).toEqual([]) + + // `pending` can clear without anything to watch; the session's created event answers at once. + server.emit("session.created") + + expect(await created).toBe("pty_new1") + + // A removed workspace ends a wait that would never end, quietly. + const waiting = terminal.new() + model.remove({ server: server.ref.id, directory: "/repo" }) + + expect(await waiting).toBeUndefined() + expect(server.calls.create).toHaveLength(1) + expect(toasts).toEqual([]) + }) + + // A failed close used to end in a console line, with the tab gone and the terminal still running. + test("a failed close tells the user and brings the tab back; a terminal already gone closes quietly", async () => { + const server = fakeServer() + server.terminals.set("ses_1", [info("pty_1", "Terminal 1"), info("pty_2", "Terminal 2")]) + const { model, session, stored, toasts } = setup(server) + stored.set(session.key, ["pty_1", "pty_2"]) + const terminal = model.load(session) + await flush() + + // Closing the tab let the strip drop it. + stored.set(session.key, ["pty_2"]) + server.state.removeError = new Error("boom") + terminal.close("pty_1") + terminal.close("pty_1") + await flush() + + expect(server.calls.remove).toEqual(["pty_1"]) + expect(toasts).toEqual([{ variant: "error", title: "close.failed", description: "boom" }]) + expect(stored.get(session.key)).toEqual(["pty_2", "pty_1"]) + + // A terminal that exited meanwhile is already closed. + server.terminals.set("ses_1", [info("pty_1", "Terminal 1")]) + server.state.removeError = new PtyNotFoundError({ ptyID: "pty_2", message: "gone" }) + terminal.close("pty_2") + await flush() + + expect(toasts).toHaveLength(1) + expect(terminal.all().map((item) => item.id)).toEqual(["pty_1"]) + }) + + // A server that accepted the request and never answered used to leave the panel on "Loading terminal." with the + // restored tabs hidden and no Retry. + test("a list the server never answers fails after the request budget and offers a retry", async () => { + jest.useFakeTimers() + + try { + const server = fakeServer() + server.state.list = new Promise(() => undefined) + server.terminals.set("ses_1", [info("pty_1", "Terminal 1")]) + const { model, session } = setup(server) + const terminal = model.load(session) + await ticks() + + expect(terminal.view()).toEqual({ type: "loading" }) + + jest.advanceTimersByTime(TERMINAL_REQUEST_TIMEOUT_MS) + await ticks() + + expect(terminal.view()).toEqual({ type: "failed", error: "request.timedOut" }) + expect(terminal.tabs(["pty_restored"])).toEqual(["pty_restored"]) + + server.state.list = undefined + terminal.retry() + await ticks() + + expect(terminal.all().map((item) => item.id)).toEqual(["pty_1"]) + } finally { + jest.useRealTimers() + } + }) }) diff --git a/packages/gui-plugins/src/terminal/model.ts b/packages/gui-plugins/src/terminal/model.ts index fb15d38333d..0098cfaba2f 100644 --- a/packages/gui-plugins/src/terminal/model.ts +++ b/packages/gui-plugins/src/terminal/model.ts @@ -1,27 +1,22 @@ -import { createRoot, createSignal, getOwner, runWithOwner, untrack, type Owner } from "solid-js" +import { createSignal, getOwner, runWithOwner, untrack } from "solid-js" import { unwrap } from "solid-js/store" import { Predicate, Schema } from "effect" import { ClientError, isPtyFeatureUnsupportedError, + isPtyNotFoundError, isServiceUnavailableError, - type PersistentPtyInfo, } from "@opencode/client" -import { - createKeyed, - type Layout, - type Persisted, - type ServerRef, - type SessionRef, - type Storage, - type Toasts, -} from "@opencode/plugin/gui" +import type { Layout, Persisted, ServerRef, SessionRef, Storage, Toasts } from "@opencode/plugin/gui" +import { createDispatch } from "./dispatch" import { defaultTitle, titleNumber } from "./title" import { initialState, serverAvailable, sessionConnection, + sessionCreating, sessionView, + TERMINAL_REQUEST_TIMEOUT_MS, transition, viewItems, viewTabs, @@ -51,6 +46,9 @@ const PANELS = "#terminal-panel, #review-panel" const OVERLAYS = '[role="dialog"], [role="menu"]' +// How often a new terminal checks whether the server finished creating its session. +const SESSION_POLL_MS = 250 + const importGhostty = () => import("ghostty-web").then(async (mod) => ({ mod, ghostty: await mod.Ghostty.load() })) /** The one ghostty-web load the window's terminals share; a failed load is retried by the next terminal. */ @@ -89,64 +87,216 @@ export function createTerminalModel(input: { const serverState = (id: string) => machine.servers[id] ?? initialState - /** Resolves once the requests the event started have answered and their answers were dispatched. */ - const dispatch = (server: ServerRef, event: Event): Promise => { - if (lifetime.signal.aborted) return Promise.resolve() - // Reads from reactive computations, such as `load`, must not subscribe them to every state change. - const previous = untrack(() => unwrap(machine.servers[server.id])) ?? initialState - const next = transition(previous, event) + // The live ref and driver loop of each server, and the requests in flight by attempt. + const servers = new Map void }>() + const requests = new Map>() + // The new-terminal requests a session's create answers, and the focus request it resolves. - if (next.state !== previous) setMachine((draft) => void (draft.servers[server.id] = next.state)) + const creates = new Map< + string, + { request: number; done: { promise: Promise; resolve: (id: string | undefined) => void } } + >() - return Promise.all(next.commands.map((command) => run(server, command))).then(() => undefined) + const dispatch = (server: ServerRef, event: Event) => { + if (lifetime.signal.aborted) return + const existing = servers.get(server.id) + + if (existing) { + existing.ref = server + existing.dispatch(event) + + return + } + + const created = { + ref: server, + dispatch: createDispatch({ + // Reads from reactive computations, such as `load`, must not subscribe them to every state change. + read: () => untrack(() => unwrap(machine.servers[server.id])) ?? initialState, + write: (state) => setMachine((draft) => void (draft.servers[server.id] = state)), + transition: (state, next) => { + const result = transition(state, next) + + return { state: result.state, effects: result.commands } + }, + run: (command) => run(created.ref, command), + }), + } + + servers.set(server.id, created) + created.dispatch(event) } - const run = (server: ServerRef, command: Command): Promise => { + /** + * Runs one attempt's request under the request budget. A request the machine stopped waiting for answers nothing; one + * that runs out of time fails, even when the server never ends it. + */ + const request = ( + server: ServerRef, + attempt: number, + call: (signal: AbortSignal) => Promise, + settle: { ok: (value: A) => Event; failed: (error: Error) => Event }, + ) => { + const controller = new AbortController() + const signal = AbortSignal.any([controller.signal, lifetime.signal]) + const timedOut = new Error(input.t("request.timedOut")) + const timer = setTimeout(() => controller.abort(timedOut), TERMINAL_REQUEST_TIMEOUT_MS) + const inflight = requests.get(server.id) ?? new Map() + + const ended = new Promise((_, reject) => + signal.addEventListener("abort", () => reject(signal.reason), { once: true }), + ) + + requests.set(server.id, inflight) + inflight.set(attempt, controller) + + return Promise.race([call(signal), ended]) + .then( + (value) => dispatch(server, settle.ok(value)), + (cause: unknown) => { + if (signal.aborted && signal.reason !== timedOut) return + dispatch(server, settle.failed(cause instanceof Error ? cause : new Error(String(cause)))) + }, + ) + .finally(() => { + clearTimeout(timer) + inflight.delete(attempt) + }) + } + + const run = (server: ServerRef, command: Command): Promise | void => { if (command.type === "check-capability") - return server.client.server.info().then( + return request(server, command.attempt, (signal) => server.client.server.info({ signal }), { // Servers that predate the field omit it; they still offer terminals. - (info) => - dispatch(server, { - type: "capability.checked", - attempt: command.attempt, - supported: info.capabilities?.persistentPty !== false, - }), - (error) => { + ok: (info) => ({ + type: "capability.checked", + attempt: command.attempt, + supported: info.capabilities?.persistentPty !== false, + }), + failed: (error) => { console.error("Failed to read server terminal support", error) - return dispatch(server, { - type: "capability.failed", - attempt: command.attempt, - error: error instanceof Error ? error.message : String(error), - }) + return { type: "capability.failed", attempt: command.attempt, error: error.message } }, - ) + }) if (command.type === "load-list") - return server.client.experimental.persistentPty.list({ sessionID: command.sessionID }).then( - (items) => - dispatch(server, { type: "list.loaded", sessionID: command.sessionID, attempt: command.attempt, items }), - (error) => { - if (isServiceUnavailableError(error)) - return dispatch(server, { - type: "list.unavailable", - sessionID: command.sessionID, - attempt: command.attempt, - }) - console.error("Failed to list terminals", error) + return request( + server, + command.attempt, + (signal) => server.client.experimental.persistentPty.list({ sessionID: command.sessionID }, { signal }), + { + ok: (items) => ({ type: "list.loaded", sessionID: command.sessionID, attempt: command.attempt, items }), + failed: (error) => { + if (isServiceUnavailableError(error)) + return { type: "list.unavailable", sessionID: command.sessionID, attempt: command.attempt } + console.error("Failed to list terminals", error) - return dispatch(server, { - type: "list.failed", - sessionID: command.sessionID, - attempt: command.attempt, - error: error instanceof Error ? error.message : String(error), - }) + return { type: "list.failed", sessionID: command.sessionID, attempt: command.attempt, error: error.message } + }, }, ) + if (command.type === "create-terminal") + return request(server, command.attempt, (signal) => createTerminal(server, command, signal), { + ok: (terminal) => ({ + type: "create.succeeded", + sessionID: command.sessionID, + attempt: command.attempt, + terminal, + }), + failed: (error) => ({ + type: "create.failed", + sessionID: command.sessionID, + attempt: command.attempt, + kind: outdated(error) ? "unsupported" : "failed", + error: error.message, + }), + }) + + if (command.type === "close-terminal") + return request( + server, + command.attempt, + (signal) => server.client.experimental.persistentPty.remove({ ptyID: command.ptyID }, { signal }), + { + ok: () => ({ + type: "close.succeeded", + sessionID: command.sessionID, + ptyID: command.ptyID, + attempt: command.attempt, + }), + // A terminal that already exited is closed. + failed: (error) => + isPtyNotFoundError(error) + ? { + type: "close.succeeded", + sessionID: command.sessionID, + ptyID: command.ptyID, + attempt: command.attempt, + } + : { + type: "close.failed", + sessionID: command.sessionID, + ptyID: command.ptyID, + attempt: command.attempt, + error: error.message, + }, + }, + ) + + if (command.type === "rename-terminal") + return request( + server, + command.attempt, + (signal) => + server.client.experimental.persistentPty.update({ ptyID: command.ptyID, title: command.title }, { signal }), + { + ok: (terminal) => ({ + type: "rename.succeeded", + sessionID: command.sessionID, + ptyID: command.ptyID, + attempt: command.attempt, + terminal, + }), + failed: (error) => ({ + type: "rename.failed", + sessionID: command.sessionID, + ptyID: command.ptyID, + attempt: command.attempt, + kind: isPtyFeatureUnsupportedError(error) ? "unsupported" : "failed", + error: error.message, + }), + }, + ) + + if (command.type === "abort") { + command.attempts.forEach((attempt) => requests.get(server.id)?.get(attempt)?.abort()) + + return + } + + if (command.type === "settle-create") return settleCreate(server, command) + + if (command.type === "toast-close-failed") { + input.toast.show({ variant: "error", title: input.t("close.failed"), description: command.error }) + + return + } + + if (command.type === "toast-rename-failed") { + input.toast.show( + command.kind === "unsupported" + ? { variant: "error", title: input.t("rename.unsupported") } + : { variant: "error", title: input.t("rename.failed"), description: command.error }, + ) + + return + } + const session = sessions.get(`${server.id}\n${command.sessionID}`) - if (!session) return Promise.resolve() + if (!session) return dropLegacy(session) // The strip shows only stored tabs, so terminals another client created join it at the end. const stored = new Set(input.layout.stored(session)) @@ -154,8 +304,103 @@ export function createTerminalModel(input: { command.items.forEach((info) => { if (!stored.has(info.id)) input.layout.open(input.tab(info.id), session, { tab: "append" }) }) + } - return Promise.resolve() + const createTerminal = async ( + server: ServerRef, + command: Extract, + signal: AbortSignal, + ) => { + const target = sessions.get(`${server.id}\n${command.sessionID}`) + + if (!target) throw new Error(input.t("create.failed")) + await settled(target, signal) + + // The next default title counts the terminals the server already has. + const known = + command.known ?? + (await server.client.experimental.persistentPty.list({ sessionID: target.id }, { signal }).catch(() => [])) + + const used = new Set(known.flatMap((info) => titleNumber(info.title) ?? [])) + const number = Array.from({ length: used.size + 1 }, (_, index) => index + 1).find((n) => !used.has(n)) ?? 1 + + return server.client.experimental.persistentPty.create( + { + sessionID: target.id, + args: [], + // A session that moved keeps its object; the server's record has the directory it runs in now. + cwd: server.data.session.get(target.id)?.location.directory ?? target.location?.directory, + title: defaultTitle(number), + env: {}, + }, + { signal }, + ) + } + + /** + * Resolves once the server has created the session: on its `session.created` event, or once `pending` clears. No + * signal tracks `pending`, so it is checked again on an interval. + */ + const settled = (session: SessionRef, signal: AbortSignal) => { + if (!session.pending) return Promise.resolve() + + return new Promise((resolve, reject) => { + const end = () => { + clearInterval(poll) + stop() + signal.removeEventListener("abort", abort) + } + + const done = () => { + end() + resolve() + } + + const abort = () => { + end() + reject(signal.reason) + } + + const stop = runWithOwner(owner, () => + session.server.data.on("session.created", (event) => { + if (event.data.sessionID === session.id) done() + }), + )! + + const poll = setInterval(() => { + if (!session.pending) done() + }, SESSION_POLL_MS) + + signal.addEventListener("abort", abort, { once: true }) + }) + } + + const settleCreate = (server: ServerRef, command: Extract) => { + const key = `${server.id}\n${command.sessionID}` + const waiting = creates.get(key) + creates.delete(key) + + if (command.outcome.ok) { + const id = command.outcome.terminal.id + const target = sessions.get(key) + + if (target) selection(target).set({ id }) + + if (waiting) dispatchFocus({ type: "pending.resolved", request: waiting.request, id }) + waiting?.done.resolve(id) + + return + } + + if (waiting) dispatchFocus({ type: "pending.failed", request: waiting.request }) + + if (command.outcome.kind === "unsupported") + input.toast.show({ variant: "error", title: input.t("create.unsupported") }) + + if (command.outcome.kind === "failed") + input.toast.show({ variant: "error", title: input.t("create.failed"), description: command.outcome.error }) + + waiting?.done.resolve(undefined) } const available = (server: ServerRef) => { @@ -176,30 +421,26 @@ export function createTerminalModel(input: { // server, so they belong to the model. const stops = runWithOwner(owner, () => [ // The event stream opens with server.connected, so this runs for the first connection and every reconnect. - data.on("server.connected", () => void dispatch(server, { type: "server.connected" })), - data.on( - "persistent-pty.added", - (event) => void dispatch(server, { type: "session.refresh", sessionID: event.data.sessionID }), + data.on("server.connected", () => dispatch(server, { type: "server.connected" })), + data.on("persistent-pty.added", (event) => + dispatch(server, { type: "session.refresh", sessionID: event.data.sessionID }), ), - data.on( - "persistent-pty.removed", - (event) => void dispatch(server, { type: "session.refresh", sessionID: event.data.sessionID }), + data.on("persistent-pty.removed", (event) => + dispatch(server, { type: "session.refresh", sessionID: event.data.sessionID }), ), - data.on( - "persistent-pty.updated", - (event) => - void dispatch(server, { - type: "terminal.updated", - sessionID: event.data.sessionID, - terminal: event.data.terminal, - }), + data.on("persistent-pty.updated", (event) => + dispatch(server, { + type: "terminal.updated", + sessionID: event.data.sessionID, + terminal: event.data.terminal, + }), ), ])! watched.set(server.id, { data, stop: () => stops.forEach((stop) => stop()) }) // Checks now rather than waiting for the stream: it may have connected already, and an answer a previous // instance was waiting for never arrives here. - void dispatch(server, { type: "server.connected" }) + dispatch(server, { type: "server.connected" }) } const selection = (session: SessionRef) => { @@ -289,104 +530,33 @@ export function createTerminalModel(input: { select(id: string) { selection(session()).set({ id }) }, - /** Creates a terminal and selects it; resolves its id, or undefined when the server refuses. */ - async new() { + /** A new terminal is being created; another request joins it. */ + creating: () => sessionCreating(serverState(initial.server.id), initial.id), + /** + * Creates a terminal and selects it; resolves its id, or undefined when the server refuses. A request while one + * is being created joins it and resolves the same id. + */ + new() { const target = session() const request = dispatchFocus({ type: "request.pending", session: key, overlay: overlay() }).request + const done = creates.get(key)?.done ?? Promise.withResolvers() - await settled(target, owner) + // The create can answer before dispatch returns, so its waiter is registered first. + creates.set(key, { request, done }) + dispatch(target.server, { type: "create.requested", sessionID: target.id }) - // The next default title counts the terminals the server already has. - const known = - view().type === "ready" - ? all() - : await target.server.client.experimental.persistentPty.list({ sessionID: target.id }).catch(() => all()) - - const used = new Set(known.flatMap((info) => titleNumber(info.title) ?? [])) - const number = Array.from({ length: used.size + 1 }, (_, index) => index + 1).find((n) => !used.has(n)) ?? 1 - - const info = await target.server.client.experimental.persistentPty - .create({ - sessionID: target.id, - args: [], - // A session that moved keeps its object; the server's record has the directory it runs in now. - cwd: target.server.data.session.get(target.id)?.location.directory ?? target.location?.directory, - title: defaultTitle(number), - env: {}, - }) - .catch((error) => { - dispatchFocus({ type: "pending.failed", request }) - - // A server without the persistent terminal routes answers with a status its API never declares for them. - const outdated = - error instanceof ClientError && - error.reason === "UnexpectedStatus" && - Predicate.hasProperty(error.cause, "status") && - (error.cause.status === 404 || error.cause.status === 405) - - input.toast.show( - outdated - ? { variant: "error", title: input.t("create.unsupported") } - : { - variant: "error", - title: input.t("create.failed"), - description: error instanceof Error ? error.message : String(error), - }, - ) - - return undefined - }) - - if (!info) return undefined - - void dispatch(target.server, { type: "terminal.created", sessionID: target.id, terminal: info }) - await dispatch(target.server, { type: "session.refresh", sessionID: target.id }) - selection(target).set({ id: info.id }) - dispatchFocus({ type: "pending.resolved", request, id: info.id }) - - return info.id + return done.promise }, /** Removes the terminal on the server; its tab leaves once the server lists it no longer. */ - async close(id: string) { - const target = session() - - await target.server.client.experimental.persistentPty - .remove({ ptyID: id }) - .then(() => dispatch(target.server, { type: "session.refresh", sessionID: target.id })) - .catch((error) => console.error("Failed to close terminal", error)) + close(id: string) { + dispatch(session().server, { type: "close.requested", sessionID: initial.id, ptyID: id }) }, /** * Sets the terminal's title on the server; `null` clears it, so the shell's or the default title shows again. - * The current title, or clearing a terminal never renamed, sends nothing. + * The title already shown, or clearing a terminal never renamed, sends nothing. */ - async rename(id: string, title: string | null) { - const target = session() - const previous = get(id) - - if (!previous || title === previous.title || (title === null && previous.userTitle === null)) return - - const replace = (terminal: PersistentPtyInfo) => - void dispatch(target.server, { type: "terminal.replaced", sessionID: target.id, terminal }) - - // Only a new title is known before the server answers; a cleared one shows whatever the server falls back to. - if (title !== null) replace({ ...previous, title, userTitle: title }) - - await target.server.client.experimental.persistentPty - .update({ ptyID: id, title }) - .then(replace) - .catch((error) => { - // A newer title from the server stays. - if (title !== null && get(id)?.userTitle === title) replace(previous) - input.toast.show( - isPtyFeatureUnsupportedError(error) - ? { variant: "error", title: input.t("rename.unsupported") } - : { - variant: "error", - title: input.t("rename.failed"), - description: error instanceof Error ? error.message : String(error), - }, - ) - }) + rename(id: string, title: string | null) { + dispatch(session().server, { type: "rename.requested", sessionID: initial.id, ptyID: id, title }) }, requestFocus(id: string) { dispatchFocus({ type: "request.target", session: key, id, overlay: overlay() }) @@ -411,7 +581,7 @@ export function createTerminalModel(input: { load(session: SessionRef) { sessions.set(session.key, session) watch(session.server) - void dispatch(session.server, { type: "session.loaded", sessionID: session.id }) + dispatch(session.server, { type: "session.loaded", sessionID: session.id }) const existing = handles.get(session.key) @@ -450,9 +620,11 @@ export function createTerminalModel(input: { const server = removed[0]?.server - if (server) void dispatch(server, { type: "sessions.removed", sessionIDs: removed.map((session) => session.id) }) + if (server) dispatch(server, { type: "sessions.removed", sessionIDs: removed.map((session) => session.id) }) }, dispose() { + // Ends the changes in flight first, so a pending new-terminal request resolves and gives up its focus request. + servers.forEach((entry) => dispatch(entry.ref, { type: "instance.stopped" })) lifetime.abort() watched.forEach((entry) => entry.stop()) watched.clear() @@ -478,21 +650,12 @@ function inOverlay(node: EventTarget | null) { return node instanceof Element && node.closest(OVERLAYS) !== null } -/** Resolves once the server has created the session. */ -function settled(session: SessionRef, owner: Owner | null) { - if (!session.pending) return Promise.resolve() - - return new Promise((resolve) => - createRoot( - (dispose) => - createKeyed( - () => !session.pending, - () => { - dispose() - resolve() - }, - ), - owner, - ), +/** A server without the persistent terminal routes answers with a status its API never declares for them. */ +function outdated(error: Error) { + return ( + error instanceof ClientError && + error.reason === "UnexpectedStatus" && + Predicate.hasProperty(error.cause, "status") && + (error.cause.status === 404 || error.cause.status === 405) ) } diff --git a/packages/gui-plugins/src/terminal/panel.tsx b/packages/gui-plugins/src/terminal/panel.tsx index 668c7b1b44f..d3ff8fd63cf 100644 --- a/packages/gui-plugins/src/terminal/panel.tsx +++ b/packages/gui-plugins/src/terminal/panel.tsx @@ -207,7 +207,7 @@ export default function TerminalPanel(props: {
{plugin.t("list.failed")}
{view().error}
- @@ -242,7 +242,7 @@ export default function TerminalPanel(props: { void terminal().rename(info.id, title)} + rename={(title) => terminal().rename(info.id, title)} /> )} @@ -262,6 +262,7 @@ export default function TerminalPanel(props: { } variant="ghost" + disabled={terminal().creating()} onClick={() => void terminal().new()} aria-label={plugin.t("command.new")} /> diff --git a/packages/gui-plugins/src/terminal/renderer.tsx b/packages/gui-plugins/src/terminal/renderer.tsx index 8bd3f5e5403..f068e187f62 100644 --- a/packages/gui-plugins/src/terminal/renderer.tsx +++ b/packages/gui-plugins/src/terminal/renderer.tsx @@ -147,7 +147,7 @@ const setup: Plugin.Setup = (ctx) => { const target = current() const id = target?.terminal.active() - if (target && id) void target.terminal.close(id) + if (target && id) target.terminal.close(id) }, }), ) @@ -215,8 +215,8 @@ const setup: Plugin.Setup = (ctx) => { label={label()} title={info()?.title ?? ""} // A tab shown for a list that failed has no terminal to rename yet. - rename={info() ? (title) => void terminal().rename(id, title) : undefined} - close={() => void terminal().close(id)} + rename={info() ? (title) => terminal().rename(id, title) : undefined} + close={() => terminal().close(id)} /> ), } @@ -274,7 +274,7 @@ const setup: Plugin.Setup = (ctx) => { tabs.get(input.session.key)?.delete(input.tab.id) - if (terminal.get(input.tab.id)) void terminal.close(input.tab.id) + if (terminal.get(input.tab.id)) terminal.close(input.tab.id) }, // Before the server lists the session's terminals, a restored tab is selected on trust; `active` honours the // selection once it is listed. diff --git a/packages/gui-plugins/src/terminal/tab.tsx b/packages/gui-plugins/src/terminal/tab.tsx index f94d814a49c..572b41b23cc 100644 --- a/packages/gui-plugins/src/terminal/tab.tsx +++ b/packages/gui-plugins/src/terminal/tab.tsx @@ -22,7 +22,7 @@ export function TerminalTab(props: { const label = () => terminalTabLabel({ title: props.terminal.title, t: plugin.t }) - const close = () => void props.session.close(props.terminal.id) + const close = () => props.session.close(props.terminal.id) const focus = () => { if (rename.editing()) return diff --git a/packages/gui-plugins/src/updater/action.ts b/packages/gui-plugins/src/updater/action.ts index 684b29574d4..5737bffda2e 100644 --- a/packages/gui-plugins/src/updater/action.ts +++ b/packages/gui-plugins/src/updater/action.ts @@ -1,7 +1,21 @@ import type { UpdaterState } from "@opencode/plugin/gui/updater" +import type { WindowUpdaterState } from "./window-machine" -/** The one update action the settings row and the titlebar pill offer for a state. */ -export function updaterAction(state: UpdaterState | undefined) { +/** + * The one update action the settings row and the titlebar pill offer for a state. The window's own check or install + * shows as running at once, before main reports it, so a second click has nothing to start. + */ +export function updaterAction(state: UpdaterState | undefined, windowState?: WindowUpdaterState) { + const action = mainAction(state) + + if (windowState?.status === "checking" && action.run === "check") return { label: "action.checking" as const } + + if (windowState?.status === "installing" && action.run === "install") return { label: "action.installing" as const } + + return action +} + +function mainAction(state: UpdaterState | undefined) { if (!state) return { label: "action.checkNow" as const } switch (state.status) { diff --git a/packages/gui-plugins/src/updater/actions.tsx b/packages/gui-plugins/src/updater/actions.tsx index 6083cf476a0..e810b80823d 100644 --- a/packages/gui-plugins/src/updater/actions.tsx +++ b/packages/gui-plugins/src/updater/actions.tsx @@ -10,22 +10,23 @@ type Client = IpcClient type Context = SetupContext -/** Restarts into the staged update. A beta build moving to stable confirms the installer download first. */ +/** + * Restarts into the staged update; resolves once main took the install or refused it. A beta build moving to stable + * confirms the installer download first, and resolves once the dialog shows. + */ export function install(ctx: Context, client: Client) { const download = () => client.install().catch((cause: unknown) => requestFailed(ctx, cause)) const state = client.state() - if (state?.status !== "download-required") { - void download() - - return - } + if (state?.status !== "download-required") return download() ctx.dialogs.open( (dialog) => , { replace: true }, ) + + return Promise.resolve() } export async function check(ctx: Context, client: Client) { @@ -55,7 +56,7 @@ export async function check(ctx: Context, client: Client) { description: ctx.t("dialog.ready.message", { version: state.version }), persistent: true, actions: [ - { label: ctx.t("dialog.restart"), variant: "primary", onClick: () => install(ctx, client) }, + { label: ctx.t("dialog.restart"), variant: "primary", onClick: () => void install(ctx, client) }, { label: ctx.t("dialog.later"), variant: "secondary", onClick: "dismiss" }, ], }) diff --git a/packages/gui-plugins/src/updater/renderer.tsx b/packages/gui-plugins/src/updater/renderer.tsx index b344dbd6ad1..47b03edf1b2 100644 --- a/packages/gui-plugins/src/updater/renderer.tsx +++ b/packages/gui-plugins/src/updater/renderer.tsx @@ -1,9 +1,16 @@ import { showToast } from "@opencode/ui/toast" -import { lazy, onCleanup, Suspense } from "solid-js" +import { createSignal, lazy, onCleanup, Suspense, untrack } from "solid-js" import { Command, createKeyed, SettingsPage, TitlebarItem, type SetupContext, type Plugin } from "@opencode/plugin/gui" import { onIdle } from "@opencode/plugin/gui/builtin" import { Updater } from "@opencode/plugin/gui/updater" import { updaterAction } from "./action" +import { + ACTION_CHUNK_TIMEOUT_MS, + initialWindowUpdater, + transitionWindowUpdater, + type WindowUpdaterEffect, + type WindowUpdaterEvent, +} from "./window-machine" import { webUpdates } from "./web" import { initWhatsNew, @@ -23,14 +30,47 @@ const setup: Plugin.Setup = (ctx) => { const updater = ctx.main(Updater) // Undefined until main's updater is up and has sent its state, and again while it is gone. const state = () => updater.state() + const [windowState, setWindowState] = createSignal(initialWindowUpdater) - const act = (name: "check" | "install") => { - if (state()) return void import("./actions").then((module) => module[name](ctx, updater)) + const dispatchWindow = (event: WindowUpdaterEvent) => { + const next = transitionWindowUpdater(untrack(windowState), event) - // Not loaded yet, or not active (failed, or not on this platform): nothing can check or install. - showToast({ title: ctx.t("common.requestFailed") }) + if (next.state !== untrack(windowState)) setWindowState(next.state) + next.effects.forEach(runWindow) } + const runWindow = (effect: WindowUpdaterEffect) => { + if (effect.type === "toast-failed") + return void showToast({ title: ctx.t("common.requestFailed"), description: effect.message }) + const attempt = effect.attempt + const chunk = Promise.withResolvers() + const timer = setTimeout(() => chunk.reject(new Error()), ACTION_CHUNK_TIMEOUT_MS) + + void Promise.race([import("./actions"), chunk.promise]) + .finally(() => clearTimeout(timer)) + .then( + (module) => { + if (ctx.signal.aborted) return + + return Promise.try(() => module[effect.action](ctx, updater)).finally(() => + dispatchWindow({ type: "action.settled", attempt }), + ) + }, + (cause: unknown) => + dispatchWindow({ + type: "action.failed", + attempt, + message: cause instanceof Error && cause.message ? cause.message : undefined, + }), + ) + } + + onCleanup(() => dispatchWindow({ type: "disposed" })) + + // Main missing (not loaded yet, failed, or not on this platform) answers that nothing can check or install. + const act = (name: "check" | "install") => + dispatchWindow({ type: "action.requested", action: name, available: state() !== undefined }) + const Section = lazy(() => import("./section")) // Settings rows are small; load them while idle so settings opens without a blank row. onCleanup(onIdle(() => void Section.preload())) @@ -45,7 +85,7 @@ const setup: Plugin.Setup = (ctx) => { return { id: "update", label: ctx.t("status.label"), - title: ctx.t(updaterAction(current).label), + title: ctx.t(updaterAction(current, windowState()).label), busy: installing, run: () => act("install"), } @@ -66,9 +106,9 @@ const setup: Plugin.Setup = (ctx) => { render: () => (
updaterAction(state())} + action={() => updaterAction(state(), windowState())} run={() => { - const run = updaterAction(state()).run + const run = updaterAction(state(), windowState()).run if (run) act(run) }} diff --git a/packages/gui-plugins/src/updater/web-driver.ts b/packages/gui-plugins/src/updater/web-driver.ts index ac67015f08a..781083c20e2 100644 --- a/packages/gui-plugins/src/updater/web-driver.ts +++ b/packages/gui-plugins/src/updater/web-driver.ts @@ -1,4 +1,4 @@ -import { initialWebUpdate, transitionWebUpdate } from "./web-machine" +import { CHECK_TIMEOUT_MS, initialWebUpdate, transitionWebUpdate } from "./web-machine" import type { WebUpdateEffect, WebUpdateEvent, WebUpdateState, WebWorker } from "./web-machine" /** How often an open, visible window asks the server for a newer build. */ @@ -10,7 +10,12 @@ const RESET_CHANNEL = "opencode.updater.reset" /** How long `applying` waits for the new worker to take over before reloading anyway. */ const APPLY_TIMEOUT_MS = 10_000 -type Current = { state: WebUpdateState; registration?: ServiceWorkerRegistration; disposed: boolean } +type Current = { + state: WebUpdateState + registration?: ServiceWorkerRegistration + /** The deadlines of the check in flight and of the takeover. */ + timers: { check?: ReturnType; apply?: ReturnType } +} export type WebUpdateReport = | { readonly result: "up-to-date" } @@ -36,7 +41,7 @@ export function startWebUpdater(options: WebUpdaterOptions) { // The app's windows share one saved copy, so a reset in one tells the others to reload. const channel = container && "BroadcastChannel" in window ? new BroadcastChannel(RESET_CHANNEL) : undefined - const current: Current = { state: initialWebUpdate, disposed: false } + const current: Current = { state: initialWebUpdate, timers: {} } const queue: WebUpdateEvent[] = [] @@ -44,7 +49,6 @@ export function startWebUpdater(options: WebUpdaterOptions) { // An event dispatched while another applies (a listener that fires synchronously) waits its turn. const dispatch = (event: WebUpdateEvent) => { - if (current.disposed) return queue.push(event) if (queue.length > 1) return @@ -58,7 +62,9 @@ export function startWebUpdater(options: WebUpdaterOptions) { if (step.state !== current.state) { current.state = step.state - options.changed(step.state) + + // The window that owns the signal is going away. + if (step.state.status !== "disposed") options.changed(step.state) } step.effects.forEach(perform) @@ -80,17 +86,22 @@ export function startWebUpdater(options: WebUpdaterOptions) { if (effect.type === "update") { const attempt = effect.attempt - return void registration.update().then( - () => dispatch({ type: "check.settled", attempt, worker: newest(registration) }), - // The browser rejects with a TypeError or DOMException, both Errors. - (error: Error) => dispatch({ type: "check.failed", attempt, message: error.message }), - ) + const timer = setTimeout(() => dispatch({ type: "check.timeout", attempt }), CHECK_TIMEOUT_MS) + current.timers.check = timer + + return void registration + .update() + .then( + () => dispatch({ type: "check.settled", attempt, worker: newest(registration) }), + // The browser rejects with a TypeError or DOMException, both Errors. + (error: Error) => dispatch({ type: "check.failed", attempt, message: error.message }), + ) + .finally(() => clearTimeout(timer)) } // Workbox's generated worker skips waiting when it receives this message. registration.waiting?.postMessage({ type: "SKIP_WAITING" }) - const timer = setTimeout(() => dispatch({ type: "apply.timeout" }), APPLY_TIMEOUT_MS) - cleanup.push(() => clearTimeout(timer)) + current.timers.apply = setTimeout(() => dispatch({ type: "apply.timeout" }), APPLY_TIMEOUT_MS) } const check = (manual: boolean) => dispatch({ type: "check.requested", manual, at: Date.now() }) @@ -119,7 +130,7 @@ export function startWebUpdater(options: WebUpdaterOptions) { }) void container.ready.then((registration) => { - if (current.disposed) return + if (current.state.status === "disposed") return current.registration = registration listen(registration, "updatefound", () => { @@ -158,7 +169,9 @@ export function startWebUpdater(options: WebUpdaterOptions) { /** Drops the saved copy of the app and loads the server's current build. */ reset: () => dispatch({ type: "reset.requested" }), dispose: () => { - current.disposed = true + dispatch({ type: "disposed" }) + clearTimeout(current.timers.check) + clearTimeout(current.timers.apply) cleanup.splice(0).forEach((run) => run()) }, } diff --git a/packages/gui-plugins/src/updater/web-machine.test.ts b/packages/gui-plugins/src/updater/web-machine.test.ts index 9e3f24c718a..bba4afba1b8 100644 --- a/packages/gui-plugins/src/updater/web-machine.test.ts +++ b/packages/gui-plugins/src/updater/web-machine.test.ts @@ -156,6 +156,49 @@ describe("web update machine", () => { expect(result.effects).toEqual([{ type: "announce" }, { type: "reset" }]) }) + // A check whose request never settled used to hold `checking` for the tab's life, so every later check, the user's + // included, joined it and Settings › Updates stayed on "Checking…". + test("a check the browser never answers times out, and the next check runs", () => { + const quiet = replay([ + { type: "registered", worker: "none" }, + auto(0), + { type: "check.timeout", attempt: 1 }, + manual(1), + ]) + + expect(quiet.state).toMatchObject({ status: "checking", attempt: 2, manual: true }) + expect(quiet.effects).toEqual([ + { type: "update", attempt: 1 }, + { type: "update", attempt: 2 }, + ]) + + // The user's check, or an automatic one the user joined, says it failed; a late answer changes nothing. + const asked = replay([ + { type: "registered", worker: "none" }, + auto(0), + manual(1), + { type: "check.timeout", attempt: 1 }, + { type: "check.settled", attempt: 1, worker: "none" }, + ]) + + expect(asked.state.status).toBe("error") + expect(asked.effects.at(-1)).toEqual({ type: "report", result: "failed", message: "" }) + }) + + test("nothing runs after the updater is disposed", () => { + const result = replay([ + { type: "registered", worker: "none" }, + auto(0), + { type: "disposed" }, + { type: "check.settled", attempt: 1, worker: "waiting" }, + { type: "controller.changed" }, + manual(1), + ]) + + expect(result.state).toEqual({ status: "disposed" }) + expect(result.effects).toEqual([{ type: "update", attempt: 1 }]) + }) + test("only the reload action runs while a build is ready; checking again offers it again", () => { const result = replay([{ type: "registered", worker: "waiting" }, auto(0), manual(0)]) diff --git a/packages/gui-plugins/src/updater/web-machine.ts b/packages/gui-plugins/src/updater/web-machine.ts index 3da7e10e309..619943bdeee 100644 --- a/packages/gui-plugins/src/updater/web-machine.ts +++ b/packages/gui-plugins/src/updater/web-machine.ts @@ -24,6 +24,7 @@ type Meta = { * - `applying`: the newer build was asked to take over; every window reloads once it does. * - `reloading`: this window is reloading into the newer build (or a reset). * - `error`: the user's last check or download failed. + * - `disposed`: the window let the updater go; nothing runs after it. */ export type WebUpdateState = | { readonly status: "unavailable" } @@ -35,12 +36,15 @@ export type WebUpdateState = | (Meta & { readonly status: "applying" }) | { readonly status: "reloading" } | (Meta & { readonly status: "error"; readonly message: string }) + | { readonly status: "disposed" } export type WebUpdateEvent = | { readonly type: "registered"; readonly worker: WebWorker } | { readonly type: "check.requested"; readonly manual: boolean; readonly at: number } | { readonly type: "check.settled"; readonly attempt: number; readonly worker: WebWorker } | { readonly type: "check.failed"; readonly attempt: number; readonly message: string } + /** The browser's update request ran past `CHECK_TIMEOUT_MS` without settling. */ + | { readonly type: "check.timeout"; readonly attempt: number } | { readonly type: "worker.installing" } | { readonly type: "worker.installed" } | { readonly type: "worker.redundant" } @@ -50,9 +54,10 @@ export type WebUpdateEvent = /** Another window dropped the saved app, including the files this window still loads. */ | { readonly type: "reset.elsewhere" } | { readonly type: "controller.changed" } + | { readonly type: "disposed" } export type WebUpdateEffect = - /** Ask the server for a newer service worker. */ + /** Ask the server for a newer service worker; `check.timeout` answers once `CHECK_TIMEOUT_MS` passes. */ | { readonly type: "update"; readonly attempt: number } /** Tell the waiting worker to take over now. */ | { readonly type: "skip-waiting" } @@ -70,9 +75,19 @@ export type WebUpdateTransition = { readonly state: WebUpdateState; readonly eff /** Automatic checks (focus, network, timer) closer together than this ask once. A user's check always runs. */ export const AUTO_CHECK_SPACING_MS = 60_000 +/** + * How long a check waits for the browser's update request, as the desktop updater does. The service worker's request + * has no deadline of its own, and a check in flight holds every later one. + */ +export const CHECK_TIMEOUT_MS = 60_000 + export const initialWebUpdate: WebUpdateState = { status: "unavailable" } export function transitionWebUpdate(state: WebUpdateState, event: WebUpdateEvent): WebUpdateTransition { + if (state.status === "disposed") return unchanged(state) + + if (event.type === "disposed") return unchanged({ status: "disposed" }) + if (state.status === "reloading") return unchanged(state) // Another window applied the update or dropped the saved app, or this one applied it: the old build's files are @@ -98,15 +113,19 @@ export function transitionWebUpdate(state: WebUpdateState, event: WebUpdateEvent return found(meta, event.worker, state.manual) case "check.failed": + case "check.timeout": { if (state.status !== "checking" || event.attempt !== state.attempt) return unchanged(state) // An automatic check fails whenever the device is offline; only the user's own check says so. if (!state.manual) return unchanged({ ...meta, status: "idle" }) + const message = event.type === "check.failed" ? event.message : "" return { - state: { ...meta, status: "error", message: event.message }, - effects: [{ type: "report", result: "failed", message: event.message }], + state: { ...meta, status: "error", message }, + effects: [{ type: "report", result: "failed", message }], } + } + case "worker.installing": if (state.status === "downloading" || state.status === "ready" || state.status === "applying") return unchanged(state) @@ -153,6 +172,7 @@ export function webUpdateAction(state: WebUpdateState) { case "reloading": return { label: "action.reloading" as const } case "unavailable": + case "disposed": return { label: "action.checkNow" as const } default: return { label: "action.checkNow" as const, run: "check" as const } @@ -165,7 +185,7 @@ export function webUpdatePending(state: WebUpdateState) { } function checkRequested( - state: Exclude, + state: Exclude, event: Extract, ): WebUpdateTransition { if (state.status === "checking" || state.status === "downloading") diff --git a/packages/gui-plugins/src/updater/window-machine.test.ts b/packages/gui-plugins/src/updater/window-machine.test.ts new file mode 100644 index 00000000000..803d66650de --- /dev/null +++ b/packages/gui-plugins/src/updater/window-machine.test.ts @@ -0,0 +1,86 @@ +import { describe, expect, test } from "bun:test" +import { updaterAction } from "./action" +import { + initialWindowUpdater, + transitionWindowUpdater, + type WindowUpdaterEffect, + type WindowUpdaterEvent, +} from "./window-machine" + +function replay(events: readonly WindowUpdaterEvent[]) { + const effects: WindowUpdaterEffect[] = [] + + const state = events.reduce((current, event) => { + const step = transitionWindowUpdater(current, event) + effects.push(...step.effects) + + return step.state + }, initialWindowUpdater) + + return { state, effects } +} + +const ask = (action: "check" | "install", available = true): WindowUpdaterEvent => ({ + type: "action.requested", + action, + available, +}) + +describe("window updater machine", () => { + // Checking from the menu and then Settings before main answered used to show two "Checking…" toasts and two results. + test("a second check joins the one in flight, and the next check after it runs", () => { + const result = replay([ask("check"), ask("check"), { type: "action.settled", attempt: 1 }, ask("check")]) + + expect(result.effects).toEqual([ + { type: "run", action: "check", attempt: 1 }, + { type: "run", action: "check", attempt: 2 }, + ]) + expect(result.state).toEqual({ status: "checking", attempt: 2 }) + }) + + test("Settings shows the window's check as running before main reports it", () => { + const checking = replay([ask("check")]).state + + expect(updaterAction({ status: "idle" }, checking)).toEqual({ label: "action.checking" }) + expect(updaterAction({ status: "idle" }, initialWindowUpdater)).toEqual({ label: "action.checkNow", run: "check" }) + }) + + test("an install asked for during a check takes over, and the check's answer settles nothing", () => { + const result = replay([ + ask("check"), + ask("install"), + { type: "action.settled", attempt: 1 }, + ask("install"), + ask("check"), + ]) + + expect(result.effects).toEqual([ + { type: "run", action: "check", attempt: 1 }, + { type: "run", action: "install", attempt: 2 }, + ]) + expect(result.state).toEqual({ status: "installing", attempt: 2 }) + }) + + // A chunk that failed to load used to leave the click unanswered, with an unhandled rejection. + test("an actions chunk that fails or stalls says so and lets the user try again", () => { + const result = replay([ask("check"), { type: "action.failed", attempt: 1, message: "chunk" }, ask("check")]) + + expect(result.effects).toEqual([ + { type: "run", action: "check", attempt: 1 }, + { type: "toast-failed", message: "chunk" }, + { type: "run", action: "check", attempt: 2 }, + ]) + }) + + test("without main's updater a request answers that it failed, unless one already runs", () => { + expect(replay([ask("check", false)]).effects).toEqual([{ type: "toast-failed" }]) + expect(replay([ask("check"), ask("check", false)]).effects).toEqual([{ type: "run", action: "check", attempt: 1 }]) + }) + + test("nothing runs after the window lets the updater go", () => { + const result = replay([ask("check"), { type: "disposed" }, { type: "action.failed", attempt: 1 }, ask("check")]) + + expect(result.state).toEqual({ status: "disposed" }) + expect(result.effects).toEqual([{ type: "run", action: "check", attempt: 1 }]) + }) +}) diff --git a/packages/gui-plugins/src/updater/window-machine.ts b/packages/gui-plugins/src/updater/window-machine.ts new file mode 100644 index 00000000000..85ad9c6d24b --- /dev/null +++ b/packages/gui-plugins/src/updater/window-machine.ts @@ -0,0 +1,73 @@ +/** How long loading the actions chunk may take, so a missing or stalled chunk never holds the window's update actions. */ +export const ACTION_CHUNK_TIMEOUT_MS = 15_000 + +/** + * The update action this window asked for. Main's updater state says what the app is doing; this says what the window + * is doing about it, so a second request joins the first instead of showing a second "Checking…" toast and result. + * - `idle`: nothing asked; `issued` is the last attempt handed out. + * - `checking`: a check runs, with its toast, until it reports what it found. + * - `installing`: the install is being handed to main, or its download confirmed. + * - `disposed`: the window let the updater go. + */ +export type WindowUpdaterState = + | { readonly status: "idle"; readonly issued: number } + | { readonly status: "checking"; readonly attempt: number } + | { readonly status: "installing"; readonly attempt: number } + | { readonly status: "disposed" } + +export type WindowUpdaterEvent = + /** The user asked from the menu, a command, Settings or the titlebar. `available` is false while main is missing. */ + | { readonly type: "action.requested"; readonly action: "check" | "install"; readonly available: boolean } + /** The action finished and answered the user. */ + | { readonly type: "action.settled"; readonly attempt: number } + /** The actions chunk failed to load or ran out of time, so the action never started. */ + | { readonly type: "action.failed"; readonly attempt: number; readonly message?: string } + | { readonly type: "disposed" } + +export type WindowUpdaterEffect = + /** Loads the actions chunk under `ACTION_CHUNK_TIMEOUT_MS` and runs the action. */ + | { readonly type: "run"; readonly action: "check" | "install"; readonly attempt: number } + | { readonly type: "toast-failed"; readonly message?: string } + +export type WindowUpdaterTransition = { + readonly state: WindowUpdaterState + readonly effects: readonly WindowUpdaterEffect[] +} + +export const initialWindowUpdater: WindowUpdaterState = { status: "idle", issued: 0 } + +export function transitionWindowUpdater(state: WindowUpdaterState, event: WindowUpdaterEvent): WindowUpdaterTransition { + if (state.status === "disposed") return unchanged(state) + + if (event.type === "disposed") return unchanged({ status: "disposed" }) + + if (event.type === "action.requested") { + if (state.status === "installing") return unchanged(state) + + // Not loaded yet, or not active (failed, or not on this platform): nothing can check or install. + if (!event.available) + return state.status === "idle" ? unchanged(state, [{ type: "toast-failed" }]) : unchanged(state) + + if (state.status === "checking" && event.action === "check") return unchanged(state) + // An install asked for during a check takes over; the check's answer no longer settles anything. + const attempt = (state.status === "idle" ? state.issued : state.attempt) + 1 + + return { + state: event.action === "check" ? { status: "checking", attempt } : { status: "installing", attempt }, + effects: [{ type: "run", action: event.action, attempt }], + } + } + + if (state.status === "idle" || event.attempt !== state.attempt) return unchanged(state) + + if (event.type === "action.settled") return unchanged({ status: "idle", issued: state.attempt }) + + return { + state: { status: "idle", issued: state.attempt }, + effects: [{ type: "toast-failed", message: event.message }], + } +} + +function unchanged(state: WindowUpdaterState, effects: readonly WindowUpdaterEffect[] = []): WindowUpdaterTransition { + return { state, effects } +} diff --git a/packages/gui-plugins/src/wsl/add-machine.test.ts b/packages/gui-plugins/src/wsl/add-machine.test.ts new file mode 100644 index 00000000000..9ee760839d6 --- /dev/null +++ b/packages/gui-plugins/src/wsl/add-machine.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, test } from "bun:test" +import { initialWslAdd, transitionWslAdd, type WslAddEffect, type WslAddEvent, type WslAddState } from "./add-machine" + +type Module = { readonly name: string } + +const replay = (events: readonly WslAddEvent[]) => + events.reduce<{ state: WslAddState; effects: WslAddEffect[] }>( + (last, event) => { + const next = transitionWslAdd(last.state, event) + + return { state: next.state, effects: [...last.effects, ...next.effects] } + }, + { state: initialWslAdd, effects: [] }, + ) + +const module: Module = { name: "dialog" } + +describe("wsl add dialog machine", () => { + // A double click used to load the chunk twice and open two dialogs; a failed chunk left the click unanswered. + test("a second click joins the dialog loading, and a failed load says so and lets the next click retry", () => { + const result = replay([ + { type: "add.requested" }, + { type: "add.requested" }, + { type: "add.failed", attempt: 1, error: "chunk" }, + { type: "add.loaded", attempt: 1, module }, + { type: "add.requested" }, + { type: "add.loaded", attempt: 2, module }, + ]) + + expect(result.effects).toEqual([ + { type: "load", attempt: 1 }, + { type: "toast-failed", error: "chunk" }, + { type: "load", attempt: 2 }, + { type: "open", module }, + ]) + }) + + test("a chunk that loads after the plugin went away opens nothing", () => { + const result = replay([{ type: "add.requested" }, { type: "disposed" }, { type: "add.loaded", attempt: 1, module }]) + + expect(result.state).toEqual({ status: "disposed" }) + expect(result.effects).toEqual([{ type: "load", attempt: 1 }]) + }) +}) diff --git a/packages/gui-plugins/src/wsl/add-machine.ts b/packages/gui-plugins/src/wsl/add-machine.ts new file mode 100644 index 00000000000..c1050f00a50 --- /dev/null +++ b/packages/gui-plugins/src/wsl/add-machine.ts @@ -0,0 +1,56 @@ +// The "Add WSL server" dialog, whose chunk loads on the first click. Each load takes the next attempt number, so a load +// the plugin's cleanup overtook never opens its dialog. + +/** How long the dialog's chunk may take to load before the click fails. */ +export const WSL_DIALOG_LOAD_TIMEOUT_MS = 15_000 + +/** + * - `idle`: no dialog loading; `issued` is the last attempt handed out. + * - `loading`: the chunk loads; a second click joins it. + * - `disposed`: the plugin instance went away. + */ +export type WslAddState = + | { readonly status: "idle"; readonly issued: number } + | { readonly status: "loading"; readonly attempt: number } + | { readonly status: "disposed" } + +export type WslAddEvent = + | { readonly type: "add.requested" } + | { readonly type: "add.loaded"; readonly attempt: number; readonly module: M } + /** The chunk failed to load or ran out of time. */ + | { readonly type: "add.failed"; readonly attempt: number; readonly error: string } + | { readonly type: "disposed" } + +export type WslAddEffect = + | { readonly type: "load"; readonly attempt: number } + | { readonly type: "open"; readonly module: M } + | { readonly type: "toast-failed"; readonly error: string } + +export type WslAddTransition = { readonly state: WslAddState; readonly effects: readonly WslAddEffect[] } + +export const initialWslAdd: WslAddState = { status: "idle", issued: 0 } + +export function transitionWslAdd(state: WslAddState, event: WslAddEvent): WslAddTransition { + if (state.status === "disposed") return unchanged(state) + + if (event.type === "disposed") return unchanged({ status: "disposed" }) + + if (event.type === "add.requested") { + // A second click joins the dialog already loading. + if (state.status === "loading") return unchanged(state) + const attempt = state.issued + 1 + + return { state: { status: "loading", attempt }, effects: [{ type: "load", attempt }] } + } + + if (state.status !== "loading" || event.attempt !== state.attempt) return unchanged(state) + + if (event.type === "add.loaded") + return { state: { status: "idle", issued: state.attempt }, effects: [{ type: "open", module: event.module }] } + + return { state: { status: "idle", issued: state.attempt }, effects: [{ type: "toast-failed", error: event.error }] } +} + +function unchanged(state: WslAddState): WslAddTransition { + return { state, effects: [] } +} diff --git a/packages/gui-plugins/src/wsl/renderer.tsx b/packages/gui-plugins/src/wsl/renderer.tsx index 8c9d02c3fee..db6ec8e9c5b 100644 --- a/packages/gui-plugins/src/wsl/renderer.tsx +++ b/packages/gui-plugins/src/wsl/renderer.tsx @@ -1,13 +1,22 @@ import { showToast } from "@opencode/ui/toast" -import { lazy, onCleanup, Suspense } from "solid-js" +import { createSignal, lazy, onCleanup, Suspense, untrack } from "solid-js" import { MenuItem, Server, Style, type ServerEntry, type ServerState, type Plugin } from "@opencode/plugin/gui" import { onIdle } from "@opencode/plugin/gui/builtin" +import { + initialWslAdd, + transitionWslAdd, + WSL_DIALOG_LOAD_TIMEOUT_MS, + type WslAddEffect, + type WslAddEvent, +} from "./add-machine" import { Wsl, type WslServerItem } from "./contract" import { wslRowView } from "./row-view" import type definition from "./index" const loadDialog = () => import("./dialog") +type DialogModule = Awaited> + const setup: Plugin.Setup = (ctx) => { if (ctx.app.platform !== "desktop") return const ipc = ctx.use(Wsl) @@ -46,14 +55,48 @@ const setup: Plugin.Setup = (ctx) => { const byKey = (key: string) => state()?.servers.find((item) => `wsl:${item.config.distro}` === key) - const add = () => - void loadDialog().then((module) => { - if (ctx.signal.aborted) return + const [addState, setAddState] = createSignal(initialWslAdd) - if (!styled.added) ctx.add(Style, module.css) - styled.added = true - dialog.open(() => ) - }) + const dispatchAdd = (event: WslAddEvent) => { + const next = transitionWslAdd(untrack(addState), event) + + if (next.state !== untrack(addState)) setAddState(next.state) + next.effects.forEach(runAdd) + } + + const runAdd = (effect: WslAddEffect) => { + if (effect.type === "toast-failed") + return void showToast({ + variant: "error", + title: ctx.t("common.requestFailed"), + description: effect.error || undefined, + }) + + if (effect.type === "load") { + const attempt = effect.attempt + const deadline = Promise.withResolvers() + const timer = setTimeout(() => deadline.reject(new Error()), WSL_DIALOG_LOAD_TIMEOUT_MS) + + // A chunk that fails or never loads, such as after an update replaced the app's files, answers the click. + return void Promise.race([loadDialog(), deadline.promise]) + .finally(() => clearTimeout(timer)) + .then( + (module) => dispatchAdd({ type: "add.loaded", attempt, module }), + (cause: unknown) => + dispatchAdd({ type: "add.failed", attempt, error: cause instanceof Error ? cause.message : String(cause) }), + ) + } + + const module = effect.module + + if (!styled.added) ctx.add(Style, module.css) + styled.added = true + dialog.open(() => ) + } + + onCleanup(() => dispatchAdd({ type: "disposed" })) + + const add = () => dispatchAdd({ type: "add.requested" }) const entry = (item: WslServerItem): ServerEntry => { const runtime = item.runtime