Skip to content

Commit a366128

Browse files
authored
fix(app): prevent terminal recovery loops (#25710)
1 parent 9f708e7 commit a366128

5 files changed

Lines changed: 122 additions & 27 deletions

File tree

packages/app/src/components/terminal.tsx

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -480,15 +480,21 @@ export const Terminal = (props: TerminalProps) => {
480480
})
481481

482482
const connectToken = async () => {
483-
const result = await client.pty.connectToken(
484-
{ ptyID: id },
485-
{
486-
throwOnError: false,
487-
headers: { "x-opencode-ticket": "1" },
488-
},
489-
)
483+
const result = await client.pty
484+
.connectToken(
485+
{ ptyID: id },
486+
{
487+
throwOnError: false,
488+
headers: { "x-opencode-ticket": "1" },
489+
},
490+
)
491+
.catch((err: unknown) => {
492+
if (err instanceof Error && err.message.includes("Request is not supported")) return
493+
throw err
494+
})
495+
if (!result) return
490496
if (result.response.status === 200 && result.data?.ticket) return result.data.ticket
491-
if ((result.response.status === 404 || result.response.status === 405) && password) return
497+
if (result.response.status === 404 || result.response.status === 405) return
492498
if (result.response.status === 403)
493499
throw new Error("PTY connect ticket rejected by origin or CSRF checks. Check the server CORS config.")
494500
throw new Error(`PTY connect ticket failed with ${result.response.status}`)

packages/app/src/context/terminal.test.ts

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import { beforeAll, describe, expect, mock, test } from "bun:test"
22

3-
let getWorkspaceTerminalCacheKey: (dir: string) => string
3+
type ServerKey = Parameters<typeof import("./terminal").getTerminalServerScope>[1]
4+
5+
let getWorkspaceTerminalCacheKey: (dir: string, scope?: string) => string
6+
let getTerminalServerScope: typeof import("./terminal").getTerminalServerScope
47
let getLegacyTerminalStorageKeys: (dir: string, legacySessionID?: string) => string[]
58
let migrateTerminalState: (value: unknown) => unknown
69

@@ -17,6 +20,7 @@ beforeAll(async () => {
1720
}))
1821
const mod = await import("./terminal")
1922
getWorkspaceTerminalCacheKey = mod.getWorkspaceTerminalCacheKey
23+
getTerminalServerScope = mod.getTerminalServerScope
2024
getLegacyTerminalStorageKeys = mod.getLegacyTerminalStorageKeys
2125
migrateTerminalState = mod.migrateTerminalState
2226
})
@@ -25,6 +29,42 @@ describe("getWorkspaceTerminalCacheKey", () => {
2529
test("uses workspace-only directory cache key", () => {
2630
expect(getWorkspaceTerminalCacheKey("/repo")).toBe("/repo:__workspace__")
2731
})
32+
33+
test("can include a server scope", () => {
34+
expect(getWorkspaceTerminalCacheKey("/repo", "wsl:Debian")).toBe("wsl:Debian:/repo:__workspace__")
35+
})
36+
})
37+
38+
describe("getTerminalServerScope", () => {
39+
test("preserves local server keys", () => {
40+
expect(
41+
getTerminalServerScope(
42+
{ type: "sidecar", variant: "base", http: { url: "http://127.0.0.1:4096" } },
43+
"sidecar" as ServerKey,
44+
),
45+
).toBeUndefined()
46+
expect(
47+
getTerminalServerScope(
48+
{ type: "http", http: { url: "http://localhost:4096" } },
49+
"http://localhost:4096" as ServerKey,
50+
),
51+
).toBeUndefined()
52+
expect(
53+
getTerminalServerScope({ type: "http", http: { url: "http://[::1]:4096" } }, "http://[::1]:4096" as ServerKey),
54+
).toBeUndefined()
55+
})
56+
57+
test("scopes non-local server keys", () => {
58+
expect(
59+
getTerminalServerScope(
60+
{ type: "sidecar", variant: "wsl", distro: "Debian", http: { url: "http://127.0.0.1:4096" } },
61+
"wsl:Debian" as ServerKey,
62+
),
63+
).toBe("wsl:Debian" as ServerKey)
64+
expect(
65+
getTerminalServerScope({ type: "http", http: { url: "https://example.com" } }, "https://example.com" as ServerKey),
66+
).toBe("https://example.com" as ServerKey)
67+
})
2868
})
2969

3070
describe("getLegacyTerminalStorageKeys", () => {

packages/app/src/context/terminal.tsx

Lines changed: 47 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { batch, createEffect, createMemo, createRoot, on, onCleanup } from "soli
44
import { useParams } from "@solidjs/router"
55
import { useSDK } from "./sdk"
66
import type { Platform } from "./platform"
7+
import { ServerConnection, useServer } from "./server"
78
import { defaultTitle, titleNumber } from "./terminal-title"
89
import { Persist, persisted, removePersisted } from "@/utils/persist"
910

@@ -82,10 +83,26 @@ export function migrateTerminalState(value: unknown) {
8283
}
8384
}
8485

85-
export function getWorkspaceTerminalCacheKey(dir: string) {
86+
export function getWorkspaceTerminalCacheKey(dir: string, scope?: string) {
87+
if (scope) return `${scope}:${dir}:${WORKSPACE_KEY}`
8688
return `${dir}:${WORKSPACE_KEY}`
8789
}
8890

91+
export function getTerminalServerScope(conn: ServerConnection.Any | undefined, key: ServerConnection.Key) {
92+
if (!conn) return
93+
if (conn.type === "sidecar" && conn.variant === "base") return
94+
if (conn.type === "http") {
95+
try {
96+
const url = new URL(conn.http.url)
97+
if (url.hostname === "localhost" || url.hostname === "127.0.0.1" || url.hostname === "::1" || url.hostname === "[::1]")
98+
return
99+
} catch {
100+
return key
101+
}
102+
}
103+
return key
104+
}
105+
89106
export function getLegacyTerminalStorageKeys(dir: string, legacySessionID?: string) {
90107
if (!legacySessionID) return [`${dir}/terminal.v1`]
91108
return [`${dir}/terminal/${legacySessionID}.v1`, `${dir}/terminal.v1`]
@@ -110,15 +127,21 @@ const trimTerminal = (pty: LocalPTY) => {
110127
}
111128
}
112129

113-
export function clearWorkspaceTerminals(dir: string, sessionIDs?: string[], platform?: Platform) {
114-
const key = getWorkspaceTerminalCacheKey(dir)
130+
export function clearWorkspaceTerminals(
131+
dir: string,
132+
sessionIDs?: string[],
133+
platform?: Platform,
134+
scope?: string,
135+
) {
136+
const key = getWorkspaceTerminalCacheKey(dir, scope)
115137
for (const cache of caches) {
116138
const entry = cache.get(key)
117139
entry?.value.clear()
118140
}
119141

120-
void removePersisted(Persist.workspace(dir, "terminal"), platform)
142+
void removePersisted(Persist.workspace(dir, scope ? `terminal:${scope}` : "terminal"), platform)
121143

144+
if (scope) return
122145
const legacy = new Set(getLegacyTerminalStorageKeys(dir))
123146
for (const id of sessionIDs ?? []) {
124147
for (const key of getLegacyTerminalStorageKeys(dir, id)) {
@@ -130,12 +153,17 @@ export function clearWorkspaceTerminals(dir: string, sessionIDs?: string[], plat
130153
}
131154
}
132155

133-
function createWorkspaceTerminalSession(sdk: ReturnType<typeof useSDK>, dir: string, legacySessionID?: string) {
134-
const legacy = getLegacyTerminalStorageKeys(dir, legacySessionID)
156+
function createWorkspaceTerminalSession(
157+
sdk: ReturnType<typeof useSDK>,
158+
dir: string,
159+
legacySessionID?: string,
160+
scope?: string,
161+
) {
162+
const legacy = scope ? [] : getLegacyTerminalStorageKeys(dir, legacySessionID)
135163

136164
const [store, setStore, _, ready] = persisted(
137165
{
138-
...Persist.workspace(dir, "terminal", legacy),
166+
...Persist.workspace(dir, scope ? `terminal:${scope}` : "terminal", legacy),
139167
migrate: migrateTerminalState,
140168
},
141169
createStore<{
@@ -357,8 +385,12 @@ export const { use: useTerminal, provider: TerminalProvider } = createSimpleCont
357385
gate: false,
358386
init: () => {
359387
const sdk = useSDK()
388+
const server = useServer()
360389
const params = useParams()
361390
const cache = new Map<string, TerminalCacheEntry>()
391+
const scope = createMemo(() => {
392+
return getTerminalServerScope(server.current, server.key)
393+
})
362394

363395
caches.add(cache)
364396
onCleanup(() => caches.delete(cache))
@@ -382,9 +414,9 @@ export const { use: useTerminal, provider: TerminalProvider } = createSimpleCont
382414
}
383415
}
384416

385-
const loadWorkspace = (dir: string, legacySessionID?: string) => {
417+
const loadWorkspace = (dir: string, legacySessionID: string | undefined, serverScope: string | undefined) => {
386418
// Terminals are workspace-scoped so tabs persist while switching sessions in the same directory.
387-
const key = getWorkspaceTerminalCacheKey(dir)
419+
const key = getWorkspaceTerminalCacheKey(dir, serverScope)
388420
const existing = cache.get(key)
389421
if (existing) {
390422
cache.delete(key)
@@ -393,7 +425,7 @@ export const { use: useTerminal, provider: TerminalProvider } = createSimpleCont
393425
}
394426

395427
const entry = createRoot((dispose) => ({
396-
value: createWorkspaceTerminalSession(sdk, dir, legacySessionID),
428+
value: createWorkspaceTerminalSession(sdk, dir, legacySessionID, serverScope),
397429
dispose,
398430
}))
399431

@@ -402,16 +434,16 @@ export const { use: useTerminal, provider: TerminalProvider } = createSimpleCont
402434
return entry.value
403435
}
404436

405-
const workspace = createMemo(() => loadWorkspace(params.dir!, params.id))
437+
const workspace = createMemo(() => loadWorkspace(params.dir!, params.id, scope()))
406438

407439
createEffect(
408440
on(
409-
() => ({ dir: params.dir, id: params.id }),
441+
() => ({ dir: params.dir, id: params.id, scope: scope() }),
410442
(next, prev) => {
411443
if (!prev?.dir) return
412-
if (next.dir === prev.dir && next.id === prev.id) return
413-
if (next.dir === prev.dir && next.id) return
414-
loadWorkspace(prev.dir, prev.id).trimAll()
444+
if (next.dir === prev.dir && next.id === prev.id && next.scope === prev.scope) return
445+
if (next.dir === prev.dir && next.id && next.scope === prev.scope) return
446+
loadWorkspace(prev.dir, prev.id, prev.scope).trimAll()
415447
},
416448
{ defer: true },
417449
),

packages/app/src/pages/layout.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import type { DragEvent } from "@thisbeyond/solid-dnd"
3535
import { useProviders } from "@/hooks/use-providers"
3636
import { showToast, Toast, toaster } from "@opencode-ai/ui/toast"
3737
import { useGlobalSDK } from "@/context/global-sdk"
38-
import { clearWorkspaceTerminals } from "@/context/terminal"
38+
import { clearWorkspaceTerminals, getTerminalServerScope } from "@/context/terminal"
3939
import { dropSessionCaches, pickSessionCacheEvictions } from "@/context/global-sync/session-cache"
4040
import {
4141
clearSessionPrefetchInflight,
@@ -1557,6 +1557,7 @@ export default function Layout(props: ParentProps) {
15571557
directory,
15581558
sessions.map((s) => s.id),
15591559
platform,
1560+
getTerminalServerScope(server.current, server.key),
15601561
)
15611562
await globalSDK.client.instance.dispose({ directory }).catch(() => undefined)
15621563

packages/app/src/pages/session/terminal-panel.tsx

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export function TerminalPanel() {
3737
const [store, setStore] = createStore({
3838
autoCreated: false,
3939
activeDraggable: undefined as string | undefined,
40+
recovered: {} as Record<string, boolean>,
4041
view: typeof window === "undefined" ? 1000 : (window.visualViewport?.height ?? window.innerHeight),
4142
})
4243

@@ -145,6 +146,21 @@ export function TerminalPanel() {
145146
const all = terminal.all
146147
const ids = createMemo(() => all().map((pty) => pty.id))
147148

149+
const recoverTerminal = (key: string, id: string, clone: (id: string) => Promise<void>) => {
150+
if (store.recovered[key]) return
151+
setStore("recovered", key, true)
152+
void clone(id)
153+
}
154+
155+
const terminalRecoveryKey = (pty: { id: string; title: string; titleNumber: number }) => {
156+
return String(pty.titleNumber || pty.title || pty.id)
157+
}
158+
159+
const markTerminalConnected = (key: string, id: string, trim: (id: string) => void) => {
160+
setStore("recovered", key, false)
161+
trim(id)
162+
}
163+
148164
const handleTerminalDragStart = (event: unknown) => {
149165
const id = getDraggableId(event)
150166
if (!id) return
@@ -280,9 +296,9 @@ export function TerminalPanel() {
280296
<Terminal
281297
pty={pty()}
282298
autoFocus={opened()}
283-
onConnect={() => ops.trim(id)}
299+
onConnect={() => markTerminalConnected(terminalRecoveryKey(pty()), id, ops.trim)}
284300
onCleanup={ops.update}
285-
onConnectError={() => ops.clone(id)}
301+
onConnectError={() => recoverTerminal(terminalRecoveryKey(pty()), id, ops.clone)}
286302
/>
287303
</div>
288304
)}

0 commit comments

Comments
 (0)