From a366128a93869ff5868223d3b4116764220b4266 Mon Sep 17 00:00:00 2001 From: Luke Parker <10430890+Hona@users.noreply.github.com> Date: Mon, 4 May 2026 23:24:57 +1000 Subject: [PATCH] fix(app): prevent terminal recovery loops (#25710) --- packages/app/src/components/terminal.tsx | 22 ++++--- packages/app/src/context/terminal.test.ts | 42 ++++++++++++- packages/app/src/context/terminal.tsx | 62 ++++++++++++++----- packages/app/src/pages/layout.tsx | 3 +- .../app/src/pages/session/terminal-panel.tsx | 20 +++++- 5 files changed, 122 insertions(+), 27 deletions(-) diff --git a/packages/app/src/components/terminal.tsx b/packages/app/src/components/terminal.tsx index 7bcc02d62d..d8ed63b8d2 100644 --- a/packages/app/src/components/terminal.tsx +++ b/packages/app/src/components/terminal.tsx @@ -480,15 +480,21 @@ export const Terminal = (props: TerminalProps) => { }) const connectToken = async () => { - const result = await client.pty.connectToken( - { ptyID: id }, - { - throwOnError: false, - headers: { "x-opencode-ticket": "1" }, - }, - ) + const result = await client.pty + .connectToken( + { ptyID: id }, + { + throwOnError: false, + headers: { "x-opencode-ticket": "1" }, + }, + ) + .catch((err: unknown) => { + if (err instanceof Error && err.message.includes("Request is not supported")) return + throw err + }) + if (!result) return if (result.response.status === 200 && result.data?.ticket) return result.data.ticket - if ((result.response.status === 404 || result.response.status === 405) && password) return + if (result.response.status === 404 || result.response.status === 405) return if (result.response.status === 403) throw new Error("PTY connect ticket rejected by origin or CSRF checks. Check the server CORS config.") throw new Error(`PTY connect ticket failed with ${result.response.status}`) diff --git a/packages/app/src/context/terminal.test.ts b/packages/app/src/context/terminal.test.ts index 6e07e03124..623303fbf4 100644 --- a/packages/app/src/context/terminal.test.ts +++ b/packages/app/src/context/terminal.test.ts @@ -1,6 +1,9 @@ import { beforeAll, describe, expect, mock, test } from "bun:test" -let getWorkspaceTerminalCacheKey: (dir: string) => string +type ServerKey = Parameters[1] + +let getWorkspaceTerminalCacheKey: (dir: string, scope?: string) => string +let getTerminalServerScope: typeof import("./terminal").getTerminalServerScope let getLegacyTerminalStorageKeys: (dir: string, legacySessionID?: string) => string[] let migrateTerminalState: (value: unknown) => unknown @@ -17,6 +20,7 @@ beforeAll(async () => { })) const mod = await import("./terminal") getWorkspaceTerminalCacheKey = mod.getWorkspaceTerminalCacheKey + getTerminalServerScope = mod.getTerminalServerScope getLegacyTerminalStorageKeys = mod.getLegacyTerminalStorageKeys migrateTerminalState = mod.migrateTerminalState }) @@ -25,6 +29,42 @@ describe("getWorkspaceTerminalCacheKey", () => { test("uses workspace-only directory cache key", () => { expect(getWorkspaceTerminalCacheKey("/repo")).toBe("/repo:__workspace__") }) + + test("can include a server scope", () => { + expect(getWorkspaceTerminalCacheKey("/repo", "wsl:Debian")).toBe("wsl:Debian:/repo:__workspace__") + }) +}) + +describe("getTerminalServerScope", () => { + test("preserves local server keys", () => { + expect( + getTerminalServerScope( + { type: "sidecar", variant: "base", http: { url: "http://127.0.0.1:4096" } }, + "sidecar" as ServerKey, + ), + ).toBeUndefined() + expect( + getTerminalServerScope( + { type: "http", http: { url: "http://localhost:4096" } }, + "http://localhost:4096" as ServerKey, + ), + ).toBeUndefined() + expect( + getTerminalServerScope({ type: "http", http: { url: "http://[::1]:4096" } }, "http://[::1]:4096" as ServerKey), + ).toBeUndefined() + }) + + test("scopes non-local server keys", () => { + expect( + getTerminalServerScope( + { type: "sidecar", variant: "wsl", distro: "Debian", http: { url: "http://127.0.0.1:4096" } }, + "wsl:Debian" as ServerKey, + ), + ).toBe("wsl:Debian" as ServerKey) + expect( + getTerminalServerScope({ type: "http", http: { url: "https://example.com" } }, "https://example.com" as ServerKey), + ).toBe("https://example.com" as ServerKey) + }) }) describe("getLegacyTerminalStorageKeys", () => { diff --git a/packages/app/src/context/terminal.tsx b/packages/app/src/context/terminal.tsx index 31d2d6e04c..0dcebd567d 100644 --- a/packages/app/src/context/terminal.tsx +++ b/packages/app/src/context/terminal.tsx @@ -4,6 +4,7 @@ import { batch, createEffect, createMemo, createRoot, on, onCleanup } from "soli import { useParams } from "@solidjs/router" import { useSDK } from "./sdk" import type { Platform } from "./platform" +import { ServerConnection, useServer } from "./server" import { defaultTitle, titleNumber } from "./terminal-title" import { Persist, persisted, removePersisted } from "@/utils/persist" @@ -82,10 +83,26 @@ export function migrateTerminalState(value: unknown) { } } -export function getWorkspaceTerminalCacheKey(dir: string) { +export function getWorkspaceTerminalCacheKey(dir: string, scope?: string) { + if (scope) return `${scope}:${dir}:${WORKSPACE_KEY}` return `${dir}:${WORKSPACE_KEY}` } +export function getTerminalServerScope(conn: ServerConnection.Any | undefined, key: ServerConnection.Key) { + if (!conn) return + if (conn.type === "sidecar" && conn.variant === "base") return + if (conn.type === "http") { + try { + const url = new URL(conn.http.url) + if (url.hostname === "localhost" || url.hostname === "127.0.0.1" || url.hostname === "::1" || url.hostname === "[::1]") + return + } catch { + return key + } + } + return key +} + export function getLegacyTerminalStorageKeys(dir: string, legacySessionID?: string) { if (!legacySessionID) return [`${dir}/terminal.v1`] return [`${dir}/terminal/${legacySessionID}.v1`, `${dir}/terminal.v1`] @@ -110,15 +127,21 @@ const trimTerminal = (pty: LocalPTY) => { } } -export function clearWorkspaceTerminals(dir: string, sessionIDs?: string[], platform?: Platform) { - const key = getWorkspaceTerminalCacheKey(dir) +export function clearWorkspaceTerminals( + dir: string, + sessionIDs?: string[], + platform?: Platform, + scope?: string, +) { + const key = getWorkspaceTerminalCacheKey(dir, scope) for (const cache of caches) { const entry = cache.get(key) entry?.value.clear() } - void removePersisted(Persist.workspace(dir, "terminal"), platform) + void removePersisted(Persist.workspace(dir, scope ? `terminal:${scope}` : "terminal"), platform) + if (scope) return const legacy = new Set(getLegacyTerminalStorageKeys(dir)) for (const id of sessionIDs ?? []) { for (const key of getLegacyTerminalStorageKeys(dir, id)) { @@ -130,12 +153,17 @@ export function clearWorkspaceTerminals(dir: string, sessionIDs?: string[], plat } } -function createWorkspaceTerminalSession(sdk: ReturnType, dir: string, legacySessionID?: string) { - const legacy = getLegacyTerminalStorageKeys(dir, legacySessionID) +function createWorkspaceTerminalSession( + sdk: ReturnType, + dir: string, + legacySessionID?: string, + scope?: string, +) { + const legacy = scope ? [] : getLegacyTerminalStorageKeys(dir, legacySessionID) const [store, setStore, _, ready] = persisted( { - ...Persist.workspace(dir, "terminal", legacy), + ...Persist.workspace(dir, scope ? `terminal:${scope}` : "terminal", legacy), migrate: migrateTerminalState, }, createStore<{ @@ -357,8 +385,12 @@ export const { use: useTerminal, provider: TerminalProvider } = createSimpleCont gate: false, init: () => { const sdk = useSDK() + const server = useServer() const params = useParams() const cache = new Map() + const scope = createMemo(() => { + return getTerminalServerScope(server.current, server.key) + }) caches.add(cache) onCleanup(() => caches.delete(cache)) @@ -382,9 +414,9 @@ export const { use: useTerminal, provider: TerminalProvider } = createSimpleCont } } - const loadWorkspace = (dir: string, legacySessionID?: string) => { + const loadWorkspace = (dir: string, legacySessionID: string | undefined, serverScope: string | undefined) => { // Terminals are workspace-scoped so tabs persist while switching sessions in the same directory. - const key = getWorkspaceTerminalCacheKey(dir) + const key = getWorkspaceTerminalCacheKey(dir, serverScope) const existing = cache.get(key) if (existing) { cache.delete(key) @@ -393,7 +425,7 @@ export const { use: useTerminal, provider: TerminalProvider } = createSimpleCont } const entry = createRoot((dispose) => ({ - value: createWorkspaceTerminalSession(sdk, dir, legacySessionID), + value: createWorkspaceTerminalSession(sdk, dir, legacySessionID, serverScope), dispose, })) @@ -402,16 +434,16 @@ export const { use: useTerminal, provider: TerminalProvider } = createSimpleCont return entry.value } - const workspace = createMemo(() => loadWorkspace(params.dir!, params.id)) + const workspace = createMemo(() => loadWorkspace(params.dir!, params.id, scope())) createEffect( on( - () => ({ dir: params.dir, id: params.id }), + () => ({ dir: params.dir, id: params.id, scope: scope() }), (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() + if (next.dir === prev.dir && next.id === prev.id && next.scope === prev.scope) return + if (next.dir === prev.dir && next.id && next.scope === prev.scope) return + loadWorkspace(prev.dir, prev.id, prev.scope).trimAll() }, { defer: true }, ), diff --git a/packages/app/src/pages/layout.tsx b/packages/app/src/pages/layout.tsx index 7e9e2d32aa..a08372649f 100644 --- a/packages/app/src/pages/layout.tsx +++ b/packages/app/src/pages/layout.tsx @@ -35,7 +35,7 @@ import type { DragEvent } from "@thisbeyond/solid-dnd" import { useProviders } from "@/hooks/use-providers" import { showToast, Toast, toaster } from "@opencode-ai/ui/toast" import { useGlobalSDK } from "@/context/global-sdk" -import { clearWorkspaceTerminals } from "@/context/terminal" +import { clearWorkspaceTerminals, getTerminalServerScope } from "@/context/terminal" import { dropSessionCaches, pickSessionCacheEvictions } from "@/context/global-sync/session-cache" import { clearSessionPrefetchInflight, @@ -1557,6 +1557,7 @@ export default function Layout(props: ParentProps) { directory, sessions.map((s) => s.id), platform, + getTerminalServerScope(server.current, server.key), ) await globalSDK.client.instance.dispose({ directory }).catch(() => undefined) diff --git a/packages/app/src/pages/session/terminal-panel.tsx b/packages/app/src/pages/session/terminal-panel.tsx index 2c2d9817f0..d7868d9170 100644 --- a/packages/app/src/pages/session/terminal-panel.tsx +++ b/packages/app/src/pages/session/terminal-panel.tsx @@ -37,6 +37,7 @@ export function TerminalPanel() { const [store, setStore] = createStore({ autoCreated: false, activeDraggable: undefined as string | undefined, + recovered: {} as Record, view: typeof window === "undefined" ? 1000 : (window.visualViewport?.height ?? window.innerHeight), }) @@ -145,6 +146,21 @@ export function TerminalPanel() { const all = terminal.all const ids = createMemo(() => all().map((pty) => pty.id)) + const recoverTerminal = (key: string, id: string, clone: (id: string) => Promise) => { + if (store.recovered[key]) return + setStore("recovered", key, true) + void clone(id) + } + + const terminalRecoveryKey = (pty: { id: string; title: string; titleNumber: number }) => { + return String(pty.titleNumber || pty.title || pty.id) + } + + const markTerminalConnected = (key: string, id: string, trim: (id: string) => void) => { + setStore("recovered", key, false) + trim(id) + } + const handleTerminalDragStart = (event: unknown) => { const id = getDraggableId(event) if (!id) return @@ -280,9 +296,9 @@ export function TerminalPanel() { ops.trim(id)} + onConnect={() => markTerminalConnected(terminalRecoveryKey(pty()), id, ops.trim)} onCleanup={ops.update} - onConnectError={() => ops.clone(id)} + onConnectError={() => recoverTerminal(terminalRecoveryKey(pty()), id, ops.clone)} /> )}