Răsfoiți Sursa

fix(app): prevent terminal mount from stealing focus (#36576)

Luke Parker 1 lună în urmă
părinte
comite
a1ed99278a

+ 122 - 2
packages/app/e2e/regression/terminal-composer-focus.spec.ts

@@ -1,5 +1,5 @@
 import { base64Encode } from "@opencode-ai/core/util/encode"
 import { base64Encode } from "@opencode-ai/core/util/encode"
-import { expect, test } from "@playwright/test"
+import { expect, test, type Page } from "@playwright/test"
 import { mockOpenCodeServer } from "../utils/mock-server"
 import { mockOpenCodeServer } from "../utils/mock-server"
 import { expectSessionTitle } from "../utils/waits"
 import { expectSessionTitle } from "../utils/waits"
 
 
@@ -7,10 +7,11 @@ const directory = "C:/OpenCode/TerminalComposerFocus"
 const projectID = "proj_terminal_composer_focus"
 const projectID = "proj_terminal_composer_focus"
 const sessionID = "ses_terminal_composer_focus"
 const sessionID = "ses_terminal_composer_focus"
 const ptyID = "pty_terminal_composer_focus"
 const ptyID = "pty_terminal_composer_focus"
+const newPtyID = "pty_terminal_composer_focus_new"
 
 
 test.use({ viewport: { width: 1440, height: 900 } })
 test.use({ viewport: { width: 1440, height: 900 } })
 
 
-test("routes typing to the composer unless the open terminal is focused", async ({ page }) => {
+test.beforeEach(async ({ page }) => {
   await mockOpenCodeServer(page, {
   await mockOpenCodeServer(page, {
     directory,
     directory,
     project: {
     project: {
@@ -67,7 +68,9 @@ test("routes typing to the composer unless the open terminal is focused", async
   await page.addInitScript(() => {
   await page.addInitScript(() => {
     localStorage.setItem("settings.v3", JSON.stringify({ general: { newLayoutDesigns: true } }))
     localStorage.setItem("settings.v3", JSON.stringify({ general: { newLayoutDesigns: true } }))
   })
   })
+})
 
 
+test("routes typing to the composer unless the open terminal is focused", async ({ page }) => {
   await page.goto(`/${base64Encode(directory)}/session/${sessionID}`)
   await page.goto(`/${base64Encode(directory)}/session/${sessionID}`)
   await expectSessionTitle(page, "Terminal composer focus")
   await expectSessionTitle(page, "Terminal composer focus")
 
 
@@ -87,3 +90,120 @@ test("routes typing to the composer unless the open terminal is focused", async
   await expect(composer).toBeFocused()
   await expect(composer).toBeFocused()
   await expect(composer).toHaveText("a")
   await expect(composer).toHaveText("a")
 })
 })
+
+test("keeps composer focus when a cached terminal finishes mounting", async ({ page }) => {
+  const ghostty = Promise.withResolvers<void>()
+  const release = Promise.withResolvers<void>()
+  const created = { count: 0 }
+  await page.route("**/pty", (route) => {
+    created.count += 1
+    return route.fulfill({
+      status: 200,
+      contentType: "application/json",
+      body: JSON.stringify({ id: ptyID, title: "Terminal 1" }),
+    })
+  })
+  await page.route(/ghostty-web/, async (route) => {
+    ghostty.resolve()
+    await release.promise
+    await route.continue()
+  })
+  await seedCachedTerminal(page)
+
+  await page.goto(`/${base64Encode(directory)}/session/${sessionID}`, { waitUntil: "commit" })
+  await expectSessionTitle(page, "Terminal composer focus")
+
+  const composer = page.locator('[data-component="prompt-input"]')
+  const terminal = page.locator('[data-component="terminal"]')
+  await expect(terminal).toBeVisible()
+  expect(created.count).toBe(0)
+  await ghostty.promise
+  await composer.click()
+  await expect(composer).toBeFocused()
+
+  release.resolve()
+  await expect(terminal.locator("textarea")).toHaveCount(1)
+  await page.waitForTimeout(300)
+  await expect(composer).toBeFocused()
+})
+
+test("keeps newer composer focus while an explicit terminal open finishes", async ({ page }) => {
+  const ghostty = Promise.withResolvers<void>()
+  const release = Promise.withResolvers<void>()
+  await page.route(/ghostty-web/, async (route) => {
+    ghostty.resolve()
+    await release.promise
+    await route.continue()
+  })
+
+  await page.goto(`/${base64Encode(directory)}/session/${sessionID}`)
+  await expectSessionTitle(page, "Terminal composer focus")
+
+  const composer = page.locator('[data-component="prompt-input"]')
+  const terminal = page.locator('[data-component="terminal"]')
+  await page.keyboard.press("Control+Backquote")
+  await expect(terminal).toBeVisible()
+  await ghostty.promise
+  await composer.click()
+  await expect(composer).toBeFocused()
+
+  release.resolve()
+  await expect(terminal.locator("textarea")).toHaveCount(1)
+  await page.waitForTimeout(50)
+  await expect(composer).toBeFocused()
+})
+
+test("focuses a terminal created from the new-terminal button", async ({ page }) => {
+  const created = { count: 0 }
+  await page.route("**/pty", (route) => {
+    created.count += 1
+    const next = created.count === 1 ? { id: ptyID, title: "Terminal 1" } : { id: newPtyID, title: "Terminal 2" }
+    return route.fulfill({
+      status: 200,
+      contentType: "application/json",
+      body: JSON.stringify(next),
+    })
+  })
+  await page.route(`**/pty/${newPtyID}`, (route) =>
+    route.fulfill({ status: 200, contentType: "application/json", body: "{}" }),
+  )
+  await page.route(`**/pty/${newPtyID}/connect-token*`, (route) =>
+    route.fulfill({
+      status: 200,
+      contentType: "application/json",
+      headers: { "access-control-allow-origin": "*" },
+      body: JSON.stringify({ ticket: "e2e-ticket" }),
+    }),
+  )
+  await page.routeWebSocket(new RegExp(`/pty/${newPtyID}/connect`), () => undefined)
+
+  await page.goto(`/${base64Encode(directory)}/session/${sessionID}`)
+  await expectSessionTitle(page, "Terminal composer focus")
+
+  const composer = page.locator('[data-component="prompt-input"]')
+  const terminal = page.locator('[data-component="terminal"]')
+  await page.keyboard.press("Control+Backquote")
+  await expect(terminal.locator("textarea")).toHaveCount(1)
+  await composer.click()
+  await expect(composer).toBeFocused()
+
+  await page.getByRole("button", { name: "New terminal" }).click()
+  await expect(page.getByRole("tab", { name: "Terminal 2" })).toHaveAttribute("aria-selected", "true")
+  await expect.poll(() => terminal.evaluate((element) => element.contains(document.activeElement))).toBe(true)
+})
+
+function seedCachedTerminal(page: Page) {
+  return page.addInitScript(
+    ({ terminalKey, ptyID }) => {
+      localStorage.setItem("opencode.global.dat:layout", JSON.stringify({ terminal: { height: 320, opened: true } }))
+      localStorage.setItem(
+        terminalKey,
+        JSON.stringify({
+          active: ptyID,
+          all: [{ id: ptyID, title: "Terminal 1", titleNumber: 1 }],
+        }),
+      )
+    },
+    { terminalKey: `${base64Encode(directory)}/terminal.v1`, ptyID },
+  )
+}

+ 3 - 0
packages/app/src/components/session/session-sortable-terminal-tab-v2.tsx

@@ -65,9 +65,12 @@ export function SortableTerminalTabV2(props: {
 
 
   const focus = () => {
   const focus = () => {
     if (store.editing) return
     if (store.editing) return
+    terminal.requestFocus(props.terminal.id)
     terminal.open(props.terminal.id)
     terminal.open(props.terminal.id)
     if (document.activeElement instanceof HTMLElement) document.activeElement.blur()
     if (document.activeElement instanceof HTMLElement) document.activeElement.blur()
     focusTerminalById(props.terminal.id)
     focusTerminalById(props.terminal.id)
+    const input = document.getElementById(`terminal-wrapper-${props.terminal.id}`)?.querySelector("textarea")
+    if (input === document.activeElement) terminal.consumeFocus(props.terminal.id)
   }
   }
 
 
   const edit = (e?: Event) => {
   const edit = (e?: Event) => {

+ 27 - 2
packages/app/src/components/terminal.tsx

@@ -23,6 +23,7 @@ const DEFAULT_TOGGLE_TERMINAL_KEYBIND = "ctrl+`"
 export interface TerminalProps extends ComponentProps<"div"> {
 export interface TerminalProps extends ComponentProps<"div"> {
   pty: LocalPTY
   pty: LocalPTY
   autoFocus?: boolean
   autoFocus?: boolean
+  onAutoFocus?: () => void
   onSubmit?: () => void
   onSubmit?: () => void
   onCleanup?: (pty: Partial<LocalPTY> & { id: string }) => void
   onCleanup?: (pty: Partial<LocalPTY> & { id: string }) => void
   onConnect?: () => void
   onConnect?: () => void
@@ -185,7 +186,15 @@ export const Terminal = (props: TerminalProps) => {
   const authToken = connection.type === "http" ? connection.authToken : false
   const authToken = connection.type === "http" ? connection.authToken : false
   const sameOrigin = new URL(url, location.href).origin === location.origin
   const sameOrigin = new URL(url, location.href).origin === location.origin
   let container!: HTMLDivElement
   let container!: HTMLDivElement
-  const [local, others] = splitProps(props, ["pty", "class", "classList", "autoFocus", "onConnect", "onConnectError"])
+  const [local, others] = splitProps(props, [
+    "pty",
+    "class",
+    "classList",
+    "autoFocus",
+    "onAutoFocus",
+    "onConnect",
+    "onConnectError",
+  ])
   const id = local.pty.id
   const id = local.pty.id
   const restore = typeof local.pty.buffer === "string" ? local.pty.buffer : ""
   const restore = typeof local.pty.buffer === "string" ? local.pty.buffer : ""
   const restoreSize =
   const restoreSize =
@@ -416,6 +425,7 @@ export const Terminal = (props: TerminalProps) => {
       fitAddon = fit
       fitAddon = fit
       serializeAddon = serializer
       serializeAddon = serializer
 
 
+      const active = document.activeElement
       t.open(container)
       t.open(container)
       useTerminalUiBindings({
       useTerminalUiBindings({
         container,
         container,
@@ -425,7 +435,22 @@ export const Terminal = (props: TerminalProps) => {
         handleLinkClick,
         handleLinkClick,
       })
       })
 
 
-      if (local.autoFocus !== false) focusTerminal()
+      if (local.autoFocus === true) {
+        focusTerminal()
+        local.onAutoFocus?.()
+      }
+      if (local.autoFocus !== true) {
+        const restoreFocus = () => {
+          const current = document.activeElement
+          if (current !== container && !container.contains(current)) return
+          t.blur()
+          t.textarea?.blur()
+          if (active instanceof HTMLElement && active.isConnected) active.focus()
+        }
+        restoreFocus()
+        const timer = setTimeout(restoreFocus, 0)
+        cleanups.push(() => clearTimeout(timer))
+      }
 
 
       if (typeof document !== "undefined" && document.fonts) {
       if (typeof document !== "undefined" && document.fonts) {
         void document.fonts.ready.then(scheduleFit)
         void document.fonts.ready.then(scheduleFit)

+ 68 - 5
packages/app/src/context/terminal.tsx

@@ -163,6 +163,43 @@ function createWorkspaceTerminalSession(
       all: [],
       all: [],
     }),
     }),
   )
   )
+  const [ui, setUi] = createStore({
+    focus: undefined as { request: number; id?: string; pending: boolean } | undefined,
+  })
+  const focus = { request: 0 }
+
+  const requestFocus = (id?: string, pending = false) => {
+    focus.request += 1
+    setUi("focus", { request: focus.request, id, pending })
+    return focus.request
+  }
+
+  const focusRequested = (id?: string) => {
+    if (!id) return false
+    if (!ui.focus || ui.focus.pending) return false
+    return !ui.focus.id || ui.focus.id === id
+  }
+
+  const consumeFocus = (id: string) => {
+    if (!focusRequested(id)) return
+    setUi("focus", undefined)
+  }
+
+  const cancelFocus = (request?: number) => {
+    if (request !== undefined && ui.focus?.request !== request) return
+    setUi("focus", undefined)
+  }
+
+  if (typeof document !== "undefined") {
+    const cancelOnOutsideFocus = (event: FocusEvent) => {
+      if (!ui.focus) return
+      if (!(event.target instanceof Element)) return
+      if (event.target.closest("#terminal-panel")) return
+      cancelFocus()
+    }
+    document.addEventListener("focusin", cancelOnOutsideFocus)
+    onCleanup(() => document.removeEventListener("focusin", cancelOnOutsideFocus))
+  }
 
 
   const pickNextTerminalNumber = () => {
   const pickNextTerminalNumber = () => {
     const existingTitleNumbers = new Set(
     const existingTitleNumbers = new Set(
@@ -267,23 +304,33 @@ function createWorkspaceTerminalSession(
         setStore("all", [])
         setStore("all", [])
       })
       })
     },
     },
-    new() {
+    new(options?: { focus?: boolean }) {
       const nextNumber = pickNextTerminalNumber()
       const nextNumber = pickNextTerminalNumber()
+      const focusRequest = options?.focus ? requestFocus(undefined, true) : undefined
 
 
       sdk.client.pty
       sdk.client.pty
         .create({ title: defaultTitle(nextNumber) })
         .create({ title: defaultTitle(nextNumber) })
         .then((pty: { data?: { id?: string; title?: string } }) => {
         .then((pty: { data?: { id?: string; title?: string } }) => {
           const id = pty.data?.id
           const id = pty.data?.id
-          if (!id) return
+          if (!id) {
+            if (focusRequest !== undefined) cancelFocus(focusRequest)
+            return
+          }
           const newTerminal = {
           const newTerminal = {
             id,
             id,
             title: pty.data?.title ?? defaultTitle(nextNumber),
             title: pty.data?.title ?? defaultTitle(nextNumber),
             titleNumber: nextNumber,
             titleNumber: nextNumber,
           }
           }
-          setStore("all", store.all.length, newTerminal)
-          setStore("active", id)
+          batch(() => {
+            setStore("all", store.all.length, newTerminal)
+            setStore("active", id)
+            if (focusRequest !== undefined && ui.focus?.request === focusRequest) {
+              setUi("focus", { request: focusRequest, id, pending: false })
+            }
+          })
         })
         })
         .catch((error: unknown) => {
         .catch((error: unknown) => {
+          if (focusRequest !== undefined) cancelFocus(focusRequest)
           console.error("Failed to create terminal", error)
           console.error("Failed to create terminal", error)
         })
         })
     },
     },
@@ -324,6 +371,18 @@ function createWorkspaceTerminalSession(
     open(id: string) {
     open(id: string) {
       setStore("active", id)
       setStore("active", id)
     },
     },
+    requestFocus(id?: string) {
+      requestFocus(id)
+    },
+    focusRequested(id?: string) {
+      return focusRequested(id)
+    },
+    consumeFocus(id: string) {
+      consumeFocus(id)
+    },
+    cancelFocus() {
+      cancelFocus()
+    },
     next() {
     next() {
       const index = store.all.findIndex((x) => x.id === store.active)
       const index = store.all.findIndex((x) => x.id === store.active)
       if (index === -1) return
       if (index === -1) return
@@ -442,13 +501,17 @@ export const { use: useTerminal, provider: TerminalProvider } = createSimpleCont
       ready: () => workspace().ready(),
       ready: () => workspace().ready(),
       all: () => workspace().all(),
       all: () => workspace().all(),
       active: () => workspace().active(),
       active: () => workspace().active(),
-      new: () => workspace().new(),
+      new: (options?: { focus?: boolean }) => workspace().new(options),
       update: (pty: Partial<LocalPTY> & { id: string }) => workspace().update(pty),
       update: (pty: Partial<LocalPTY> & { id: string }) => workspace().update(pty),
       trim: (id: string) => workspace().trim(id),
       trim: (id: string) => workspace().trim(id),
       trimAll: () => workspace().trimAll(),
       trimAll: () => workspace().trimAll(),
       clone: (id: string) => workspace().clone(id),
       clone: (id: string) => workspace().clone(id),
       bind: () => workspace(),
       bind: () => workspace(),
       open: (id: string) => workspace().open(id),
       open: (id: string) => workspace().open(id),
+      requestFocus: (id?: string) => workspace().requestFocus(id),
+      focusRequested: (id?: string) => workspace().focusRequested(id),
+      consumeFocus: (id: string) => workspace().consumeFocus(id),
+      cancelFocus: () => workspace().cancelFocus(),
       close: (id: string) => workspace().close(id),
       close: (id: string) => workspace().close(id),
       move: (id: string, to: number) => workspace().move(id, to),
       move: (id: string, to: number) => workspace().move(id, to),
       next: () => workspace().next(),
       next: () => workspace().next(),

+ 10 - 32
packages/app/src/pages/session/terminal-panel-v2.tsx

@@ -28,7 +28,6 @@ import { getTerminalHandoff, setTerminalHandoff } from "@/pages/session/handoff"
 import { useSessionLayout } from "@/pages/session/session-layout"
 import { useSessionLayout } from "@/pages/session/session-layout"
 
 
 export function TerminalPanelV2(props: { stacked?: boolean } = {}) {
 export function TerminalPanelV2(props: { stacked?: boolean } = {}) {
-  const delays = [120, 240]
   const layout = useLayout()
   const layout = useLayout()
   const terminal = useTerminal()
   const terminal = useTerminal()
   const sdk = useSDK()
   const sdk = useSDK()
@@ -46,6 +45,8 @@ export function TerminalPanelV2(props: { stacked?: boolean } = {}) {
   let root: HTMLDivElement | undefined
   let root: HTMLDivElement | undefined
   let tabList: HTMLDivElement | undefined
   let tabList: HTMLDivElement | undefined
 
 
+  onCleanup(() => terminal.cancelFocus())
+
   const [store, setStore] = createStore({
   const [store, setStore] = createStore({
     autoCreated: false,
     autoCreated: false,
     recovered: {} as Record<string, boolean>,
     recovered: {} as Record<string, boolean>,
@@ -94,36 +95,12 @@ export function TerminalPanelV2(props: { stacked?: boolean } = {}) {
     ),
     ),
   )
   )
 
 
-  const focus = (id: string) => {
-    focusTerminalById(id)
-
-    const frame = requestAnimationFrame(() => {
-      if (!opened()) return
-      if (terminal.active() !== id) return
-      focusTerminalById(id)
-    })
-
-    const timers = delays.map((ms) =>
-      window.setTimeout(() => {
-        if (!opened()) return
-        if (terminal.active() !== id) return
-        focusTerminalById(id)
-      }, ms),
-    )
-
-    return () => {
-      cancelAnimationFrame(frame)
-      for (const timer of timers) clearTimeout(timer)
-    }
-  }
-
   createEffect(
   createEffect(
     on(
     on(
-      () => [opened(), terminal.active()] as const,
-      ([next, id]) => {
-        if (!next || !id) return
-        const stop = focus(id)
-        onCleanup(stop)
+      () => [opened(), terminal.active(), terminal.focusRequested(terminal.active())] as const,
+      ([next, id, requested]) => {
+        if (!next || !id || !requested) return
+        focusTerminalById(id)
       },
       },
     ),
     ),
   )
   )
@@ -304,7 +281,7 @@ export function TerminalPanelV2(props: { stacked?: boolean } = {}) {
                             icon="plus-small"
                             icon="plus-small"
                             variant="ghost"
                             variant="ghost"
                             iconSize="large"
                             iconSize="large"
-                            onClick={terminal.new}
+                            onClick={() => terminal.new({ focus: true })}
                             aria-label={language.t("command.terminal.new")}
                             aria-label={language.t("command.terminal.new")}
                           />
                           />
                         </TooltipKeybind>
                         </TooltipKeybind>
@@ -326,7 +303,7 @@ export function TerminalPanelV2(props: { stacked?: boolean } = {}) {
                           icon="plus-small"
                           icon="plus-small"
                           variant="ghost"
                           variant="ghost"
                           iconSize="large"
                           iconSize="large"
-                          onClick={terminal.new}
+                          onClick={() => terminal.new({ focus: true })}
                           aria-label={language.t("command.terminal.new")}
                           aria-label={language.t("command.terminal.new")}
                         />
                         />
                       </TooltipV2>
                       </TooltipV2>
@@ -344,7 +321,8 @@ export function TerminalPanelV2(props: { stacked?: boolean } = {}) {
                           <div id={`terminal-wrapper-${id}`} class="absolute inset-0">
                           <div id={`terminal-wrapper-${id}`} class="absolute inset-0">
                             <Terminal
                             <Terminal
                               pty={pty()}
                               pty={pty()}
-                              autoFocus={opened()}
+                              autoFocus={terminal.focusRequested(id)}
+                              onAutoFocus={() => terminal.consumeFocus(id)}
                               class="!px-[14px]"
                               class="!px-[14px]"
                               onConnect={() => markTerminalConnected(terminalRecoveryKey(pty()), id, ops.trim)}
                               onConnect={() => markTerminalConnected(terminalRecoveryKey(pty()), id, ops.trim)}
                               onCleanup={ops.update}
                               onCleanup={ops.update}

+ 4 - 1
packages/app/src/pages/session/terminal-panel.tsx

@@ -38,6 +38,8 @@ export function TerminalPanel() {
   const close = () => view().terminal.close()
   const close = () => view().terminal.close()
   let root: HTMLDivElement | undefined
   let root: HTMLDivElement | undefined
 
 
+  onCleanup(() => terminal.cancelFocus())
+
   const [store, setStore] = createStore({
   const [store, setStore] = createStore({
     autoCreated: false,
     autoCreated: false,
     activeDraggable: undefined as string | undefined,
     activeDraggable: undefined as string | undefined,
@@ -285,7 +287,7 @@ export function TerminalPanel() {
                         icon="plus-small"
                         icon="plus-small"
                         variant="ghost"
                         variant="ghost"
                         iconSize="large"
                         iconSize="large"
-                        onClick={terminal.new}
+                        onClick={() => terminal.new()}
                         aria-label={language.t("command.terminal.new")}
                         aria-label={language.t("command.terminal.new")}
                       />
                       />
                     </TooltipKeybind>
                     </TooltipKeybind>
@@ -303,6 +305,7 @@ export function TerminalPanel() {
                             <Terminal
                             <Terminal
                               pty={pty()}
                               pty={pty()}
                               autoFocus={opened()}
                               autoFocus={opened()}
+                              onAutoFocus={() => terminal.consumeFocus(id)}
                               onConnect={() => markTerminalConnected(terminalRecoveryKey(pty()), id, ops.trim)}
                               onConnect={() => markTerminalConnected(terminalRecoveryKey(pty()), id, ops.trim)}
                               onCleanup={ops.update}
                               onCleanup={ops.update}
                               onConnectError={() => recoverTerminal(terminalRecoveryKey(pty()), id, ops.clone)}
                               onConnectError={() => recoverTerminal(terminalRecoveryKey(pty()), id, ops.clone)}

+ 11 - 2
packages/app/src/pages/session/use-session-commands.tsx

@@ -264,7 +264,8 @@ export const useSessionCommands = (actions: SessionCommandContext) => {
   }
   }
 
 
   const openTerminal = () => {
   const openTerminal = () => {
-    if (terminal.all().length > 0) terminal.new()
+    if (terminal.all().length > 0) terminal.new({ focus: true })
+    if (terminal.all().length === 0) terminal.requestFocus()
     view().terminal.open()
     view().terminal.open()
   }
   }
 
 
@@ -498,7 +499,15 @@ export const useSessionCommands = (actions: SessionCommandContext) => {
       title: language.t("command.terminal.toggle"),
       title: language.t("command.terminal.toggle"),
       keybind: "ctrl+`",
       keybind: "ctrl+`",
       slash: "terminal",
       slash: "terminal",
-      onSelect: () => view().terminal.toggle(),
+      onSelect: () => {
+        if (view().terminal.opened()) {
+          terminal.cancelFocus()
+          view().terminal.close()
+          return
+        }
+        terminal.requestFocus(terminal.active())
+        view().terminal.open()
+      },
     }),
     }),
     viewCommand({
     viewCommand({
       id: "review.toggle",
       id: "review.toggle",