Browse Source

fix: trim retained desktop terminal buffers (#16583)

Shoubhit Dash 5 months ago
parent
commit
6e9e027886

+ 26 - 7
packages/app/e2e/terminal/terminal-tabs.spec.ts

@@ -44,12 +44,14 @@ async function store(page: Page, key: string) {
   }, key)
 }
 
-test("terminal tab buffers persist across tab switches", async ({ page, withProject }) => {
+test("inactive terminal tab buffers persist across tab switches", async ({ page, withProject }) => {
   await withProject(async ({ directory, gotoSession }) => {
     const key = workspacePersistKey(directory, "terminal")
     const one = `E2E_TERM_ONE_${Date.now()}`
     const two = `E2E_TERM_TWO_${Date.now()}`
     const tabs = page.locator('#terminal-panel [data-slot="tabs-trigger"]')
+    const first = tabs.filter({ hasText: /Terminal 1/ }).first()
+    const second = tabs.filter({ hasText: /Terminal 2/ }).first()
 
     await gotoSession()
     await open(page)
@@ -61,22 +63,39 @@ test("terminal tab buffers persist across tab switches", async ({ page, withProj
 
     await run(page, `echo ${two}`)
 
-    await tabs
-      .filter({ hasText: /Terminal 1/ })
-      .first()
-      .click()
+    await first.click()
+    await expect(first).toHaveAttribute("aria-selected", "true")
+    await expect
+      .poll(
+        async () => {
+          const state = await store(page, key)
+          const first = state?.all.find((item) => item.titleNumber === 1)?.buffer ?? ""
+          const second = state?.all.find((item) => item.titleNumber === 2)?.buffer ?? ""
+          return {
+            first: first.includes(one),
+            second: second.includes(two),
+          }
+        },
+        { timeout: 30_000 },
+      )
+      .toEqual({ first: false, second: true })
 
+    await second.click()
+    await expect(second).toHaveAttribute("aria-selected", "true")
     await expect
       .poll(
         async () => {
           const state = await store(page, key)
           const first = state?.all.find((item) => item.titleNumber === 1)?.buffer ?? ""
           const second = state?.all.find((item) => item.titleNumber === 2)?.buffer ?? ""
-          return first.includes(one) && second.includes(two)
+          return {
+            first: first.includes(one),
+            second: second.includes(two),
+          }
         },
         { timeout: 30_000 },
       )
-      .toBe(true)
+      .toEqual({ first: true, second: false })
   })
 })
 

+ 38 - 1
packages/app/src/context/terminal.tsx

@@ -1,6 +1,6 @@
 import { createStore, produce } from "solid-js/store"
 import { createSimpleContext } from "@opencode-ai/ui/context"
-import { batch, createEffect, createMemo, createRoot, onCleanup } from "solid-js"
+import { batch, createEffect, createMemo, createRoot, on, onCleanup } from "solid-js"
 import { useParams } from "@solidjs/router"
 import { useSDK } from "./sdk"
 import type { Platform } from "./platform"
@@ -38,6 +38,16 @@ type TerminalCacheEntry = {
 
 const caches = new Set<Map<string, TerminalCacheEntry>>()
 
+const trimTerminal = (pty: LocalPTY) => {
+  if (!pty.buffer && pty.cursor === undefined && pty.scrollY === undefined) return pty
+  return {
+    ...pty,
+    buffer: undefined,
+    cursor: undefined,
+    scrollY: undefined,
+  }
+}
+
 export function clearWorkspaceTerminals(dir: string, sessionIDs?: string[], platform?: Platform) {
   const key = getWorkspaceTerminalCacheKey(dir)
   for (const cache of caches) {
@@ -188,6 +198,18 @@ function createWorkspaceTerminalSession(sdk: ReturnType<typeof useSDK>, dir: str
           console.error("Failed to update terminal", error)
         })
     },
+    trim(id: string) {
+      const index = store.all.findIndex((x) => x.id === id)
+      if (index === -1) return
+      setStore("all", index, (pty) => trimTerminal(pty))
+    },
+    trimAll() {
+      setStore("all", (all) => {
+        const next = all.map(trimTerminal)
+        if (next.every((pty, index) => pty === all[index])) return all
+        return next
+      })
+    },
     async clone(id: string) {
       const index = store.all.findIndex((x) => x.id === id)
       const pty = store.all[index]
@@ -322,12 +344,27 @@ export const { use: useTerminal, provider: TerminalProvider } = createSimpleCont
 
     const workspace = createMemo(() => loadWorkspace(params.dir!, params.id))
 
+    createEffect(
+      on(
+        () => ({ dir: params.dir, id: params.id }),
+        (next, prev) => {
+          if (!prev?.dir) return
+          if (next.dir === prev.dir && next.id === prev.id) return
+          if (next.dir === prev.dir && next.id) return
+          loadWorkspace(prev.dir, prev.id).trimAll()
+        },
+        { defer: true },
+      ),
+    )
+
     return {
       ready: () => workspace().ready(),
       all: () => workspace().all(),
       active: () => workspace().active(),
       new: () => workspace().new(),
       update: (pty: Partial<LocalPTY> & { id: string }) => workspace().update(pty),
+      trim: (id: string) => workspace().trim(id),
+      trimAll: () => workspace().trimAll(),
       clone: (id: string) => workspace().clone(id),
       open: (id: string) => workspace().open(id),
       close: (id: string) => workspace().close(id),

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

@@ -250,6 +250,7 @@ export function TerminalPanel() {
                           <div id={`terminal-wrapper-${id}`} class="absolute inset-0">
                             <Terminal
                               pty={pty()}
+                              onConnect={() => terminal.trim(id)}
                               onCleanup={terminal.update}
                               onConnectError={() => terminal.clone(id)}
                             />