Skip to content

Commit c9e9dbe

Browse files
authored
fix(app): terminal cloning without retry (#17354)
1 parent b88b323 commit c9e9dbe

5 files changed

Lines changed: 256 additions & 87 deletions

File tree

packages/app/e2e/actions.ts

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,22 @@ async function terminalID(term: Locator) {
3636
throw new Error(`Active terminal missing ${terminalAttr}`)
3737
}
3838

39+
export async function terminalConnects(page: Page, input?: { term?: Locator }) {
40+
const term = input?.term ?? page.locator(terminalSelector).first()
41+
const id = await terminalID(term)
42+
return page.evaluate((id) => {
43+
return (window as E2EWindow).__opencode_e2e?.terminal?.terminals?.[id]?.connects ?? 0
44+
}, id)
45+
}
46+
47+
export async function disconnectTerminal(page: Page, input?: { term?: Locator }) {
48+
const term = input?.term ?? page.locator(terminalSelector).first()
49+
const id = await terminalID(term)
50+
await page.evaluate((id) => {
51+
;(window as E2EWindow).__opencode_e2e?.terminal?.controls?.[id]?.disconnect?.()
52+
}, id)
53+
}
54+
3955
async function terminalReady(page: Page, term?: Locator) {
4056
const next = term ?? page.locator(terminalSelector).first()
4157
const id = await terminalID(next)
@@ -588,12 +604,19 @@ export async function seedSessionTask(
588604
.flatMap((message) => message.parts)
589605
.find((part) => {
590606
if (part.type !== "tool" || part.tool !== "task") return false
591-
if (part.state.input?.description !== input.description) return false
592-
return typeof part.state.metadata?.sessionId === "string" && part.state.metadata.sessionId.length > 0
607+
if (!("state" in part) || !part.state || typeof part.state !== "object") return false
608+
if (!("input" in part.state) || !part.state.input || typeof part.state.input !== "object") return false
609+
if (!("description" in part.state.input) || part.state.input.description !== input.description) return false
610+
if (!("metadata" in part.state) || !part.state.metadata || typeof part.state.metadata !== "object")
611+
return false
612+
if (!("sessionId" in part.state.metadata)) return false
613+
return typeof part.state.metadata.sessionId === "string" && part.state.metadata.sessionId.length > 0
593614
})
594615

595-
if (!part) return
596-
const id = part.state.metadata?.sessionId
616+
if (!part || !("state" in part) || !part.state || typeof part.state !== "object") return
617+
if (!("metadata" in part.state) || !part.state.metadata || typeof part.state.metadata !== "object") return
618+
if (!("sessionId" in part.state.metadata)) return
619+
const id = part.state.metadata.sessionId
597620
if (typeof id !== "string" || !id) return
598621
const child = await sdk.session
599622
.get({ sessionID: id })
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import type { Page } from "@playwright/test"
2+
import { disconnectTerminal, runTerminal, terminalConnects, waitTerminalReady } from "../actions"
3+
import { test, expect } from "../fixtures"
4+
import { terminalSelector } from "../selectors"
5+
import { terminalToggleKey } from "../utils"
6+
7+
async function open(page: Page) {
8+
const term = page.locator(terminalSelector).first()
9+
const visible = await term.isVisible().catch(() => false)
10+
if (!visible) await page.keyboard.press(terminalToggleKey)
11+
await waitTerminalReady(page, { term })
12+
return term
13+
}
14+
15+
test("terminal reconnects without replacing the pty", async ({ page, withProject }) => {
16+
await withProject(async ({ gotoSession }) => {
17+
const name = `OPENCODE_E2E_RECONNECT_${Date.now()}`
18+
const token = `E2E_RECONNECT_${Date.now()}`
19+
20+
await gotoSession()
21+
22+
const term = await open(page)
23+
const id = await term.getAttribute("data-pty-id")
24+
if (!id) throw new Error("Active terminal missing data-pty-id")
25+
26+
const prev = await terminalConnects(page, { term })
27+
28+
await runTerminal(page, {
29+
term,
30+
cmd: `export ${name}=${token}; echo ${token}`,
31+
token,
32+
})
33+
34+
await disconnectTerminal(page, { term })
35+
36+
await expect.poll(() => terminalConnects(page, { term }), { timeout: 15_000 }).toBeGreaterThan(prev)
37+
await expect.poll(() => term.getAttribute("data-pty-id"), { timeout: 5_000 }).toBe(id)
38+
39+
await runTerminal(page, {
40+
term,
41+
cmd: `echo $${name}`,
42+
token,
43+
timeout: 15_000,
44+
})
45+
})
46+
})

packages/app/e2e/tsconfig.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
"extends": "../tsconfig.json",
33
"compilerOptions": {
44
"noEmit": true,
5+
"rootDir": "..",
56
"types": ["node", "bun"]
67
},
7-
"include": ["./**/*.ts"]
8+
"include": ["./**/*.ts", "../src/testing/terminal.ts"]
89
}

packages/app/src/components/terminal.tsx

Lines changed: 133 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,16 @@ const debugTerminal = (...values: unknown[]) => {
6565
console.debug("[terminal]", ...values)
6666
}
6767

68+
const errorStatus = (err: unknown) => {
69+
if (!err || typeof err !== "object") return
70+
if (!("data" in err)) return
71+
const data = err.data
72+
if (!data || typeof data !== "object") return
73+
if (!("statusCode" in data)) return
74+
const status = data.statusCode
75+
return typeof status === "number" ? status : undefined
76+
}
77+
6878
const useTerminalUiBindings = (input: {
6979
container: HTMLDivElement
7080
term: Term
@@ -189,7 +199,11 @@ export const Terminal = (props: TerminalProps) => {
189199
const start =
190200
typeof local.pty.cursor === "number" && Number.isSafeInteger(local.pty.cursor) ? local.pty.cursor : undefined
191201
let cursor = start ?? 0
202+
let seek = start !== undefined ? start : restore ? -1 : 0
192203
let output: ReturnType<typeof terminalWriter> | undefined
204+
let drop: VoidFunction | undefined
205+
let reconn: ReturnType<typeof setTimeout> | undefined
206+
let tries = 0
193207

194208
const cleanup = () => {
195209
if (!cleanups.length) return
@@ -453,85 +467,135 @@ export const Terminal = (props: TerminalProps) => {
453467
}
454468

455469
const once = { value: false }
456-
let closing = false
457-
458-
const url = new URL(sdk.url + `/pty/${id}/connect`)
459-
url.searchParams.set("directory", sdk.directory)
460-
url.searchParams.set("cursor", String(start !== undefined ? start : restore ? -1 : 0))
461-
url.protocol = url.protocol === "https:" ? "wss:" : "ws:"
462-
url.username = server.current?.http.username ?? "opencode"
463-
url.password = server.current?.http.password ?? ""
464-
465-
const socket = new WebSocket(url)
466-
socket.binaryType = "arraybuffer"
467-
ws = socket
468-
469-
const handleOpen = () => {
470-
probe.connect()
471-
local.onConnect?.()
472-
scheduleSize(t.cols, t.rows)
470+
const decoder = new TextDecoder()
471+
472+
const fail = (err: unknown) => {
473+
if (disposed) return
474+
if (once.value) return
475+
once.value = true
476+
local.onConnectError?.(err)
473477
}
474-
socket.addEventListener("open", handleOpen)
475-
if (socket.readyState === WebSocket.OPEN) handleOpen()
476478

477-
const decoder = new TextDecoder()
478-
const handleMessage = (event: MessageEvent) => {
479+
const gone = () =>
480+
sdk.client.pty
481+
.get({ ptyID: id })
482+
.then(() => false)
483+
.catch((err) => {
484+
if (errorStatus(err) === 404) return true
485+
debugTerminal("failed to inspect terminal session", err)
486+
return false
487+
})
488+
489+
const retry = (err: unknown) => {
479490
if (disposed) return
480-
if (closing) return
481-
if (event.data instanceof ArrayBuffer) {
482-
const bytes = new Uint8Array(event.data)
483-
if (bytes[0] !== 0) return
484-
const json = decoder.decode(bytes.subarray(1))
485-
try {
486-
const meta = JSON.parse(json) as { cursor?: unknown }
487-
const next = meta?.cursor
488-
if (typeof next === "number" && Number.isSafeInteger(next) && next >= 0) {
489-
cursor = next
491+
if (reconn !== undefined) return
492+
493+
const ms = Math.min(250 * 2 ** Math.min(tries, 4), 4_000)
494+
reconn = setTimeout(async () => {
495+
reconn = undefined
496+
if (disposed) return
497+
if (await gone()) {
498+
if (disposed) return
499+
fail(err)
500+
return
501+
}
502+
if (disposed) return
503+
tries += 1
504+
open()
505+
}, ms)
506+
}
507+
508+
const open = () => {
509+
if (disposed) return
510+
drop?.()
511+
512+
const url = new URL(sdk.url + `/pty/${id}/connect`)
513+
url.searchParams.set("directory", sdk.directory)
514+
url.searchParams.set("cursor", String(seek))
515+
url.protocol = url.protocol === "https:" ? "wss:" : "ws:"
516+
url.username = server.current?.http.username ?? "opencode"
517+
url.password = server.current?.http.password ?? ""
518+
519+
const socket = new WebSocket(url)
520+
socket.binaryType = "arraybuffer"
521+
ws = socket
522+
523+
const handleOpen = () => {
524+
if (disposed) return
525+
tries = 0
526+
probe.connect()
527+
local.onConnect?.()
528+
scheduleSize(t.cols, t.rows)
529+
}
530+
531+
const handleMessage = (event: MessageEvent) => {
532+
if (disposed) return
533+
if (event.data instanceof ArrayBuffer) {
534+
const bytes = new Uint8Array(event.data)
535+
if (bytes[0] !== 0) return
536+
const json = decoder.decode(bytes.subarray(1))
537+
try {
538+
const meta = JSON.parse(json) as { cursor?: unknown }
539+
const next = meta?.cursor
540+
if (typeof next === "number" && Number.isSafeInteger(next) && next >= 0) {
541+
cursor = next
542+
seek = next
543+
}
544+
} catch (err) {
545+
debugTerminal("invalid websocket control frame", err)
490546
}
491-
} catch (err) {
492-
debugTerminal("invalid websocket control frame", err)
547+
return
493548
}
494-
return
549+
550+
const data = typeof event.data === "string" ? event.data : ""
551+
if (!data) return
552+
output?.push(data)
553+
cursor += data.length
554+
seek = cursor
495555
}
496556

497-
const data = typeof event.data === "string" ? event.data : ""
498-
if (!data) return
499-
output?.push(data)
500-
cursor += data.length
501-
}
502-
socket.addEventListener("message", handleMessage)
557+
const handleError = (error: Event) => {
558+
if (disposed) return
559+
debugTerminal("websocket error", error)
560+
}
503561

504-
const handleError = (error: Event) => {
505-
if (disposed) return
506-
if (closing) return
507-
if (once.value) return
508-
once.value = true
509-
console.error("WebSocket error:", error)
510-
local.onConnectError?.(error)
511-
}
512-
socket.addEventListener("error", handleError)
562+
const stop = () => {
563+
socket.removeEventListener("open", handleOpen)
564+
socket.removeEventListener("message", handleMessage)
565+
socket.removeEventListener("error", handleError)
566+
socket.removeEventListener("close", handleClose)
567+
if (ws === socket) ws = undefined
568+
if (drop === stop) drop = undefined
569+
if (socket.readyState !== WebSocket.CLOSED && socket.readyState !== WebSocket.CLOSING) socket.close(1000)
570+
}
513571

514-
const handleClose = (event: CloseEvent) => {
515-
if (disposed) return
516-
if (closing) return
517-
// Normal closure (code 1000) means PTY process exited - server event handles cleanup
518-
// For other codes (network issues, server restart), trigger error handler
519-
if (event.code !== 1000) {
520-
if (once.value) return
521-
once.value = true
522-
local.onConnectError?.(new Error(language.t("terminal.connectionLost.abnormalClose", { code: event.code })))
572+
const handleClose = (event: CloseEvent) => {
573+
if (ws === socket) ws = undefined
574+
if (drop === stop) drop = undefined
575+
socket.removeEventListener("open", handleOpen)
576+
socket.removeEventListener("message", handleMessage)
577+
socket.removeEventListener("error", handleError)
578+
socket.removeEventListener("close", handleClose)
579+
if (disposed) return
580+
if (event.code === 1000) return
581+
retry(new Error(language.t("terminal.connectionLost.abnormalClose", { code: event.code })))
523582
}
583+
584+
drop = stop
585+
socket.addEventListener("open", handleOpen)
586+
socket.addEventListener("message", handleMessage)
587+
socket.addEventListener("error", handleError)
588+
socket.addEventListener("close", handleClose)
524589
}
525-
socket.addEventListener("close", handleClose)
526-
527-
cleanups.push(() => {
528-
closing = true
529-
socket.removeEventListener("open", handleOpen)
530-
socket.removeEventListener("message", handleMessage)
531-
socket.removeEventListener("error", handleError)
532-
socket.removeEventListener("close", handleClose)
533-
if (socket.readyState !== WebSocket.CLOSED && socket.readyState !== WebSocket.CLOSING) socket.close(1000)
590+
591+
probe.control({
592+
disconnect: () => {
593+
if (!ws) return
594+
ws.close(4_000, "e2e")
595+
},
534596
})
597+
598+
open()
535599
}
536600

537601
void run().catch((err) => {
@@ -549,6 +613,8 @@ export const Terminal = (props: TerminalProps) => {
549613
disposed = true
550614
if (fitFrame !== undefined) cancelAnimationFrame(fitFrame)
551615
if (sizeTimer !== undefined) clearTimeout(sizeTimer)
616+
if (reconn !== undefined) clearTimeout(reconn)
617+
drop?.()
552618
if (ws && ws.readyState !== WebSocket.CLOSED && ws.readyState !== WebSocket.CLOSING) ws.close(1000)
553619

554620
const finalize = () => {

0 commit comments

Comments
 (0)