Browse Source

fix(app): terminal issues (#14329)

Adam 1 month ago
parent
commit
f8dad0ae17

+ 13 - 47
packages/app/src/pages/session/terminal-panel.tsx

@@ -38,34 +38,9 @@ export function TerminalPanel() {
 
   const [store, setStore] = createStore({
     autoCreated: false,
-    everOpened: false,
     activeDraggable: undefined as string | undefined,
   })
 
-  const rendered = createMemo(() => isDesktop() && (opened() || store.everOpened))
-
-  createEffect(
-    on(open, (isOpen, prev) => {
-      if (isOpen) {
-        if (!store.everOpened) setStore("everOpened", true)
-        const activeId = terminal.active()
-        if (!activeId) return
-        if (document.activeElement instanceof HTMLElement) {
-          document.activeElement.blur()
-        }
-        setTimeout(() => focusTerminalById(activeId), 0)
-        return
-      }
-
-      if (!prev) return
-      const panel = document.getElementById("terminal-panel")
-      const activeElement = document.activeElement
-      if (!panel || !(activeElement instanceof HTMLElement)) return
-      if (!panel.contains(activeElement)) return
-      activeElement.blur()
-    }),
-  )
-
   createEffect(() => {
     if (!opened()) {
       setStore("autoCreated", false)
@@ -92,7 +67,7 @@ export function TerminalPanel() {
     on(
       () => terminal.active(),
       (activeId) => {
-        if (!activeId || !open()) return
+        if (!activeId || !opened()) return
         if (document.activeElement instanceof HTMLElement) {
           document.activeElement.blur()
         }
@@ -158,32 +133,23 @@ export function TerminalPanel() {
   }
 
   return (
-    <Show when={rendered()}>
+    <Show when={open()}>
       <div
         id="terminal-panel"
         role="region"
         aria-label={language.t("terminal.title")}
-        classList={{
-          "relative w-full flex flex-col shrink-0 overflow-hidden": true,
-          "border-t border-border-weak-base": open(),
-          "pointer-events-none": !open(),
-        }}
-        style={{
-          height: `${height()}px`,
-          display: open() ? "flex" : "none",
-        }}
+        class="relative w-full flex flex-col shrink-0 border-t border-border-weak-base"
+        style={{ height: `${height()}px` }}
       >
-        <Show when={open()}>
-          <ResizeHandle
-            direction="vertical"
-            size={height()}
-            min={100}
-            max={typeof window === "undefined" ? 1000 : window.innerHeight * 0.6}
-            collapseThreshold={50}
-            onResize={layout.terminal.resize}
-            onCollapse={close}
-          />
-        </Show>
+        <ResizeHandle
+          direction="vertical"
+          size={height()}
+          min={100}
+          max={typeof window === "undefined" ? 1000 : window.innerHeight * 0.6}
+          collapseThreshold={50}
+          onResize={layout.terminal.resize}
+          onCollapse={close}
+        />
         <Show
           when={terminal.ready()}
           fallback={

+ 22 - 1
packages/opencode/src/pty/index.ts

@@ -18,12 +18,14 @@ export namespace Pty {
 
   type Socket = {
     readyState: number
+    data?: unknown
     send: (data: string | Uint8Array | ArrayBuffer) => void
     close: (code?: number, reason?: string) => void
   }
 
   type Subscriber = {
     id: number
+    token: unknown
   }
 
   const sockets = new WeakMap<object, number>()
@@ -37,6 +39,19 @@ export namespace Pty {
     return next
   }
 
+  const token = (ws: Socket) => {
+    const data = ws.data
+    if (!data || typeof data !== "object") return
+
+    const events = (data as { events?: unknown }).events
+    if (events && typeof events === "object") return events
+
+    const url = (data as { url?: unknown }).url
+    if (url && typeof url === "object") return url
+
+    return data
+  }
+
   // WebSocket control frame: 0x00 + UTF-8 JSON.
   const meta = (cursor: number) => {
     const json = JSON.stringify({ cursor })
@@ -194,6 +209,12 @@ export namespace Pty {
           session.subscribers.delete(ws)
           continue
         }
+
+        if (sub.token !== undefined && token(ws) !== sub.token) {
+          session.subscribers.delete(ws)
+          continue
+        }
+
         try {
           ws.send(chunk)
         } catch {
@@ -291,7 +312,7 @@ export namespace Pty {
     }
 
     owners.set(ws, id)
-    session.subscribers.set(ws, { id: socketId })
+    session.subscribers.set(ws, { id: socketId, token: token(ws) })
 
     const cleanup = () => {
       session.subscribers.delete(ws)

+ 46 - 0
packages/opencode/test/pty/pty-output-isolation.test.ts

@@ -18,6 +18,7 @@ describe("pty", () => {
 
           const ws = {
             readyState: 1,
+            data: { events: { connection: "a" } },
             send: (data: unknown) => {
               outA.push(typeof data === "string" ? data : Buffer.from(data as Uint8Array).toString("utf8"))
             },
@@ -30,6 +31,7 @@ describe("pty", () => {
           Pty.connect(a.id, ws as any)
 
           // Now "reuse" the same ws object for another connection.
+          ws.data = { events: { connection: "b" } }
           ws.send = (data: unknown) => {
             outB.push(typeof data === "string" ? data : Buffer.from(data as Uint8Array).toString("utf8"))
           }
@@ -51,4 +53,48 @@ describe("pty", () => {
       },
     })
   })
+
+  test("does not leak output when Bun recycles websocket objects before re-connect", async () => {
+    await using dir = await tmpdir({ git: true })
+
+    await Instance.provide({
+      directory: dir.path,
+      fn: async () => {
+        const a = await Pty.create({ command: "cat", title: "a" })
+        try {
+          const outA: string[] = []
+          const outB: string[] = []
+
+          const ws = {
+            readyState: 1,
+            data: { events: { connection: "a" } },
+            send: (data: unknown) => {
+              outA.push(typeof data === "string" ? data : Buffer.from(data as Uint8Array).toString("utf8"))
+            },
+            close: () => {
+              // no-op (simulate abrupt drop)
+            },
+          }
+
+          // Connect "a" first.
+          Pty.connect(a.id, ws as any)
+          outA.length = 0
+
+          // Simulate Bun reusing the same websocket object for another
+          // connection before the next onOpen calls Pty.connect.
+          ws.data = { events: { connection: "b" } }
+          ws.send = (data: unknown) => {
+            outB.push(typeof data === "string" ? data : Buffer.from(data as Uint8Array).toString("utf8"))
+          }
+
+          Pty.write(a.id, "AAA\n")
+          await Bun.sleep(100)
+
+          expect(outB.join("")).not.toContain("AAA")
+        } finally {
+          await Pty.remove(a.id)
+        }
+      },
+    })
+  })
 })