fix(app): show failed terminal lists with retry and reject stale answers (#61650)

Author: Hona

packages/app/e2e/regression/terminal.spec.ts

@@ -217,6 +217,35 @@ test("a dropped connection says the terminal disconnected and keeps its tab", as
   expect(await terminalText(page, screen)).toBe("still-here")
 })
 
+test("a terminal list that fails says why, and Try again lists the terminals", async ({ page }) => {
+  const { persistentPty } = 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")
+
+  // The restored session's list fails with an error other than the daemon's 503.
+  persistentPty.listFails = true
+  await page.reload()
+
+  const failure = side(page).getByRole("alert")
+  await expect(failure.getByText("Couldn't load terminals", { exact: true })).toBeVisible()
+  await expect(failure.getByText("UnexpectedStatus: 500", { exact: true })).toBeVisible()
+  // Without the list the restored tab has no title yet, but it shows and stays selected.
+  await expect(side(page).getByRole("tab", { name: "Terminal", exact: true })).toHaveAttribute("aria-selected", "true")
+
+  persistentPty.listFails = false
+  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")
+  await expect(side(page).getByText("Couldn't load terminals", { exact: true })).toHaveCount(0)
+})
+
 test("after a server restart, showing the terminal again restores it from a new snapshot", async ({ page }) => {
   const output = "before-restart\r\n"
 

packages/app/e2e/utils/mock-api.ts

@@ -277,7 +277,7 @@ const Group = HttpApiGroup.make("mock")
     HttpApiEndpoint.get("persistentPtyList", "/api/experimental/session/:sessionID/terminal", {
       params: SessionParams,
       success: Json,
-      error: PersistentPtyUnavailable,
+      error: [PersistentPtyUnavailable, MockInternal.pipe(HttpApiSchema.status(500))],
     }),
   )
   .add(

packages/app/e2e/utils/mock-server.ts

@@ -1047,7 +1047,13 @@ function mockHandlers(
         // Like the server, which also answers 204 for a shell that already ended.
         shellRemove: () => noContent,
         persistentPtyList: (ctx) =>
-          persistentPtyEnabled.pipe(Effect.map(() => ({ data: state.persistentPty.list(ctx.params.sessionID) }))),
+          persistentPtyEnabled.pipe(
+            Effect.andThen(() =>
+              state.persistentPty.listFails
+                ? Effect.fail(new MockInternal({ message: "Listing terminals failed" }))
+                : Effect.succeed({ data: state.persistentPty.list(ctx.params.sessionID) }),
+            ),
+          ),
         persistentPtyCreate: (ctx) =>
           persistentPtyEnabled.pipe(
             Effect.map(() => {

packages/app/e2e/utils/persistent-pty.ts

@@ -108,6 +108,9 @@ export type MockPersistentPty = {
   // Read by `/api/info`; tests may change both mid-test.
   capability: boolean | undefined
   renameSupported: boolean
+  // Whether listing a session's terminals answers 500, an error other than the daemon's 503. Starts false; tests may
+  // change it mid-test.
+  listFails: boolean
   // Live terminals in creation order.
   terminals: MockPersistentPtyInfo[]
   created: MockPersistentPtyInfo[]
@@ -336,6 +339,7 @@ export function createPersistentPty(
   const pty: MockPersistentPty = {
     capability: input.config?.capability,
     renameSupported: input.config?.renameSupported ?? true,
+    listFails: false,
     terminals: [],
     created: [],
     removed: [],

packages/gui-plugins/src/terminal/i18n/en.ts

@@ -5,6 +5,8 @@ export default {
   "command.new.description": "Create a new terminal tab",
   "location.open": "Open in terminal",
   loading: "Loading terminal…",
+  "list.failed": "Couldn't load terminals",
+  unavailable: "Terminals aren't available on this server",
   "title.numbered": "Terminal {{number}}",
   close: "Close terminal",
   "create.failed": "Couldn't create a terminal",

packages/gui-plugins/src/terminal/machine.test.ts

@@ -0,0 +1,269 @@
+import { describe, expect, test } from "bun:test"
+import type { PersistentPtyInfo } from "@opencode/client"
+import {
+  initialState,
+  serverAvailable,
+  sessionView,
+  tabHidden,
+  transition,
+  viewTabs,
+  type Event,
+  type State,
+  type Transition,
+} from "./machine"
+
+const info = (id: string, title = id): PersistentPtyInfo => ({
+  id,
+  title,
+  userTitle: null,
+  command: "/bin/zsh",
+  args: [],
+  cwd: "/repo",
+  status: "running",
+  pid: 1,
+  sessionID: "ses_1",
+  foregroundProcess: null,
+  size: { cols: 80, rows: 24 },
+  output: { head: 0, tail: 0 },
+})
+
+// Folds the events in order and returns the last transition.
+const replay = (events: readonly Event[], state: State = initialState) =>
+  events.reduce<Transition>((last, event) => transition(last.state, event), { state, commands: [] })
+
+// A supported server whose ses_1 listed pty_1 with attempt 2.
+const listed = () =>
+  replay([
+    { type: "server.connected" },
+    { type: "session.loaded", sessionID: "ses_1" },
+    { type: "capability.checked", attempt: 1, supported: true },
+    { type: "list.loaded", sessionID: "ses_1", attempt: 2, items: [info("pty_1")] },
+  ]).state
+
+describe("terminal machine", () => {
+  test("a list that fails with an error other than 503 shows its error and its tabs, and retries on request", () => {
+    const failed = replay([
+      { type: "session.loaded", sessionID: "ses_1" },
+      { type: "capability.checked", attempt: 1, supported: true },
+      { type: "list.failed", sessionID: "ses_1", attempt: 2, error: "boom" },
+    ])
+
+    const view = sessionView(failed.state, "ses_1")
+
+    expect(view).toEqual({ type: "failed", error: "boom" })
+    expect(tabHidden(view)).toBe(false)
+    expect(viewTabs(view, ["pty_restored"])).toEqual(["pty_restored"])
+    expect(serverAvailable(failed.state)).toBe(true)
+
+    const retried = transition(failed.state, { type: "session.retry", sessionID: "ses_1" })
+
+    expect(sessionView(retried.state, "ses_1")).toEqual({ type: "loading" })
+    expect(retried.commands).toEqual([{ type: "load-list", sessionID: "ses_1", attempt: 3 }])
+
+    const loaded = transition(retried.state, {
+      type: "list.loaded",
+      sessionID: "ses_1",
+      attempt: 3,
+      items: [info("pty_1")],
+    })
+
+    expect(sessionView(loaded.state, "ses_1")).toEqual({ type: "ready", items: [info("pty_1")] })
+    expect(loaded.commands).toEqual([{ type: "sync-tabs", sessionID: "ses_1", items: [info("pty_1")] }])
+  })
+
+  test("loading a session again changes nothing, so reactive reads never retry a failed list", () => {
+    const loading = replay([
+      { type: "session.loaded", sessionID: "ses_1" },
+      { type: "capability.checked", attempt: 1, supported: true },
+    ]).state
+
+    const failed = transition(loading, { type: "list.failed", sessionID: "ses_1", attempt: 2, error: "boom" }).state
+
+    const states = [loading, failed, listed()]
+
+    states.forEach((state) => {
+      const again = transition(state, { type: "session.loaded", sessionID: "ses_1" })
+
+      expect(again.state).toBe(state)
+      expect(again.commands).toEqual([])
+    })
+
+    // Retrying anything but a failed list repeats nothing.
+    expect(transition(loading, { type: "session.retry", sessionID: "ses_1" }).state).toBe(loading)
+  })
+
+  test("a support answer from before a reconnect is ignored", () => {
+    const rechecked = replay([
+      { type: "server.connected" },
+      { type: "session.loaded", sessionID: "ses_1" },
+      { type: "server.connected" },
+    ])
+
+    expect(rechecked.commands).toEqual([{ type: "check-capability", attempt: 2 }])
+
+    const stale = replay(
+      [
+        { type: "capability.checked", attempt: 1, supported: false },
+        { type: "capability.failed", attempt: 1, error: "offline" },
+      ],
+      rechecked.state,
+    )
+
+    expect(stale.state).toBe(rechecked.state)
+
+    const supported = transition(rechecked.state, { type: "capability.checked", attempt: 2, supported: true })
+
+    expect(supported.state.status).toBe("supported")
+    expect(supported.commands).toEqual([{ type: "load-list", sessionID: "ses_1", attempt: 3 }])
+  })
+
+  test("only the latest list request settles a session", () => {
+    const refreshing = transition(listed(), { type: "session.refresh", sessionID: "ses_1" })
+
+    expect(refreshing.commands).toEqual([{ type: "load-list", sessionID: "ses_1", attempt: 3 }])
+
+    const newer = transition(refreshing.state, { type: "session.refresh", sessionID: "ses_1" })
+
+    const stale = replay(
+      [
+        { type: "list.loaded", sessionID: "ses_1", attempt: 3, items: [] },
+        { type: "list.failed", sessionID: "ses_1", attempt: 3, error: "old" },
+        { type: "list.unavailable", sessionID: "ses_1", attempt: 3 },
+      ],
+      newer.state,
+    )
+
+    expect(stale.state).toBe(newer.state)
+    expect(sessionView(newer.state, "ses_1")).toEqual({ type: "ready", items: [info("pty_1")] })
+
+    const settled = transition(newer.state, {
+      type: "list.loaded",
+      sessionID: "ses_1",
+      attempt: 4,
+      items: [info("pty_1"), info("pty_2")],
+    })
+
+    expect(sessionView(settled.state, "ses_1")).toEqual({ type: "ready", items: [info("pty_1"), info("pty_2")] })
+  })
+
+  test("a 503 for the latest request marks the server unavailable until a reconnect finds support", () => {
+    const loading = transition(listed(), { type: "session.refresh", sessionID: "ses_1" }).state
+    const unavailable = transition(loading, { type: "list.unavailable", sessionID: "ses_1", attempt: 3 }).state
+    const view = sessionView(unavailable, "ses_1")
+
+    expect(serverAvailable(unavailable)).toBe(false)
+    expect(view).toEqual({ type: "unsupported" })
+    expect(tabHidden(view)).toBe(true)
+    expect(viewTabs(view, ["pty_1"])).toEqual(["pty_1"])
+
+    const reconnected = transition(unavailable, { type: "server.connected" })
+
+    expect(reconnected.commands).toEqual([{ type: "check-capability", attempt: 4 }])
+
+    const supported = transition(reconnected.state, { type: "capability.checked", attempt: 4, supported: true })
+
+    expect(supported.commands).toEqual([{ type: "load-list", sessionID: "ses_1", attempt: 5 }])
+  })
+
+  test("a failed refresh keeps the terminals already shown", () => {
+    const refreshing = transition(listed(), { type: "session.refresh", sessionID: "ses_1" }).state
+
+    expect(sessionView(refreshing, "ses_1")).toEqual({ type: "ready", items: [info("pty_1")] })
+
+    const failed = transition(refreshing, { type: "list.failed", sessionID: "ses_1", attempt: 3, error: "boom" })
+
+    expect(sessionView(failed.state, "ses_1")).toEqual({ type: "ready", items: [info("pty_1")] })
+  })
+
+  test("a reconnect keeps listed terminals shown, drops answers from before it, and lists every session again", () => {
+    const supported = transition(listed(), { type: "session.loaded", sessionID: "ses_2" })
+
+    expect(supported.commands).toEqual([{ type: "load-list", sessionID: "ses_2", attempt: 3 }])
+
+    const rechecking = transition(supported.state, { type: "server.connected" })
+
+    expect(rechecking.commands).toEqual([{ 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" })
+
+    const late = transition(rechecking.state, { type: "list.unavailable", sessionID: "ses_2", attempt: 3 })
+
+    expect(late.state).toBe(rechecking.state)
+
+    const relisted = transition(rechecking.state, { type: "capability.checked", attempt: 4, supported: true })
+
+    expect(relisted.commands).toEqual([
+      { type: "load-list", sessionID: "ses_1", attempt: 5 },
+      { type: "load-list", sessionID: "ses_2", attempt: 6 },
+    ])
+    expect(sessionView(relisted.state, "ses_1")).toEqual({ type: "ready", items: [info("pty_1")] })
+  })
+
+  test("a terminal update replaces a listed terminal and lists again for an unknown one", () => {
+    const state = listed()
+
+    const renamed = transition(state, {
+      type: "terminal.updated",
+      sessionID: "ses_1",
+      terminal: info("pty_1", "build"),
+    })
+
+    expect(renamed.commands).toEqual([])
+    expect(sessionView(renamed.state, "ses_1")).toEqual({ type: "ready", items: [info("pty_1", "build")] })
+
+    const unknown = transition(state, { type: "terminal.updated", sessionID: "ses_1", terminal: info("pty_9") })
+
+    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,
+    )
+    expect(transition(state, { type: "terminal.updated", sessionID: "ses_9", terminal: info("pty_9") }).state).toBe(
+      state,
+    )
+  })
+
+  test("answers for a removed session are ignored", () => {
+    const loading = replay([
+      { type: "session.loaded", sessionID: "ses_1" },
+      { type: "capability.checked", attempt: 1, supported: true },
+      { type: "sessions.removed", sessionIDs: ["ses_1"] },
+    ]).state
+
+    const late = replay(
+      [
+        { type: "list.loaded", sessionID: "ses_1", attempt: 2, items: [info("pty_1")] },
+        { type: "list.unavailable", sessionID: "ses_1", attempt: 2 },
+        { type: "list.failed", sessionID: "ses_1", attempt: 2, error: "boom" },
+      ],
+      loading,
+    )
+
+    expect(late.state).toBe(loading)
+
+    // Loading it again takes a new attempt rather than reusing the abandoned one.
+    expect(transition(loading, { type: "session.loaded", sessionID: "ses_1" }).commands).toEqual([
+      { type: "load-list", sessionID: "ses_1", attempt: 3 },
+    ])
+  })
+
+  test("a failed support check shows its error with a retry that checks again", () => {
+    const failed = replay([
+      { type: "server.connected" },
+      { type: "session.loaded", sessionID: "ses_1" },
+      { type: "capability.failed", attempt: 1, error: "offline" },
+    ]).state
+
+    const view = sessionView(failed, "ses_1")
+
+    expect(view).toEqual({ type: "failed", error: "offline" })
+    expect(tabHidden(view)).toBe(false)
+    expect(serverAvailable(failed)).toBe(false)
+
+    const retried = transition(failed, { type: "session.retry", sessionID: "ses_1" })
+
+    expect(retried.commands).toEqual([{ type: "check-capability", attempt: 2 }])
+    expect(sessionView(retried.state, "ses_1")).toEqual({ type: "loading" })
+  })
+})