diff options
| -rw-r--r-- | packages/app/src/components/terminal.tsx | 58 | ||||
| -rw-r--r-- | packages/app/src/pages/session/terminal-panel.tsx | 60 | ||||
| -rw-r--r-- | packages/opencode/src/pty/index.ts | 63 | ||||
| -rw-r--r-- | packages/opencode/src/server/routes/pty.ts | 13 | ||||
| -rw-r--r-- | packages/opencode/test/pty/pty-output-isolation.test.ts | 46 | ||||
| -rw-r--r-- | patches/[email protected] | 40 |
6 files changed, 122 insertions, 158 deletions
diff --git a/packages/app/src/components/terminal.tsx b/packages/app/src/components/terminal.tsx index 085a79613..bd7ab2447 100644 --- a/packages/app/src/components/terminal.tsx +++ b/packages/app/src/components/terminal.tsx @@ -320,8 +320,6 @@ export const Terminal = (props: TerminalProps) => { const mod = loaded.mod const g = loaded.ghostty - const once = { value: false } - const restore = typeof local.pty.buffer === "string" ? local.pty.buffer : "" const restoreSize = restore && @@ -416,20 +414,28 @@ export const Terminal = (props: TerminalProps) => { cleanups.push(() => window.removeEventListener("resize", handleResize)) } - if (restore && restoreSize) { - t.write(restore, () => { - fit.fit() - scheduleSize(t.cols, t.rows) - if (typeof local.pty.scrollY === "number") t.scrollToLine(local.pty.scrollY) - startResize() + const write = (data: string) => + new Promise<void>((resolve) => { + if (!output) { + resolve() + return + } + output.push(data) + output.flush(resolve) }) + + if (restore && restoreSize) { + await write(restore) + fit.fit() + scheduleSize(t.cols, t.rows) + if (typeof local.pty.scrollY === "number") t.scrollToLine(local.pty.scrollY) + startResize() } else { fit.fit() scheduleSize(t.cols, t.rows) if (restore) { - t.write(restore, () => { - if (typeof local.pty.scrollY === "number") t.scrollToLine(local.pty.scrollY) - }) + await write(restore) + if (typeof local.pty.scrollY === "number") t.scrollToLine(local.pty.scrollY) } startResize() } @@ -438,38 +444,32 @@ export const Terminal = (props: TerminalProps) => { // console.log("Scroll position:", ydisp) // }) + const once = { value: false } + let closing = false + const url = new URL(sdk.url + `/pty/${local.pty.id}/connect`) url.searchParams.set("directory", sdk.directory) url.searchParams.set("cursor", String(start !== undefined ? start : local.pty.buffer ? -1 : 0)) url.protocol = url.protocol === "https:" ? "wss:" : "ws:" url.username = server.current?.http.username ?? "" url.password = server.current?.http.password ?? "" + const socket = new WebSocket(url) socket.binaryType = "arraybuffer" ws = socket - cleanups.push(() => { - if (socket.readyState !== WebSocket.CLOSED && socket.readyState !== WebSocket.CLOSING) socket.close() - }) - if (disposed) { - cleanup() - return - } const handleOpen = () => { local.onConnect?.() scheduleSize(t.cols, t.rows) } socket.addEventListener("open", handleOpen) - cleanups.push(() => socket.removeEventListener("open", handleOpen)) - if (socket.readyState === WebSocket.OPEN) handleOpen() const decoder = new TextDecoder() - const handleMessage = (event: MessageEvent) => { if (disposed) return + if (closing) return if (event.data instanceof ArrayBuffer) { - // WebSocket control frame: 0x00 + UTF-8 JSON (currently { cursor }). const bytes = new Uint8Array(event.data) if (bytes[0] !== 0) return const json = decoder.decode(bytes.subarray(1)) @@ -491,20 +491,20 @@ export const Terminal = (props: TerminalProps) => { cursor += data.length } socket.addEventListener("message", handleMessage) - cleanups.push(() => socket.removeEventListener("message", handleMessage)) const handleError = (error: Event) => { if (disposed) return + if (closing) return if (once.value) return once.value = true console.error("WebSocket error:", error) local.onConnectError?.(error) } socket.addEventListener("error", handleError) - cleanups.push(() => socket.removeEventListener("error", handleError)) const handleClose = (event: CloseEvent) => { if (disposed) return + if (closing) return // Normal closure (code 1000) means PTY process exited - server event handles cleanup // For other codes (network issues, server restart), trigger error handler if (event.code !== 1000) { @@ -514,7 +514,15 @@ export const Terminal = (props: TerminalProps) => { } } socket.addEventListener("close", handleClose) - cleanups.push(() => socket.removeEventListener("close", handleClose)) + + cleanups.push(() => { + closing = true + socket.removeEventListener("open", handleOpen) + socket.removeEventListener("message", handleMessage) + socket.removeEventListener("error", handleError) + socket.removeEventListener("close", handleClose) + if (socket.readyState !== WebSocket.CLOSED && socket.readyState !== WebSocket.CLOSING) socket.close(1000) + }) } void run().catch((err) => { diff --git a/packages/app/src/pages/session/terminal-panel.tsx b/packages/app/src/pages/session/terminal-panel.tsx index 33421c386..73f61ab05 100644 --- a/packages/app/src/pages/session/terminal-panel.tsx +++ b/packages/app/src/pages/session/terminal-panel.tsx @@ -38,9 +38,34 @@ 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) @@ -67,7 +92,7 @@ export function TerminalPanel() { on( () => terminal.active(), (activeId) => { - if (!activeId || !opened()) return + if (!activeId || !open()) return if (document.activeElement instanceof HTMLElement) { document.activeElement.blur() } @@ -133,23 +158,32 @@ export function TerminalPanel() { } return ( - <Show when={open()}> + <Show when={rendered()}> <div id="terminal-panel" role="region" aria-label={language.t("terminal.title")} - class="relative w-full flex flex-col shrink-0 border-t border-border-weak-base" - style={{ height: `${height()}px` }} + 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", + }} > - <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={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> <Show when={terminal.ready()} fallback={ diff --git a/packages/opencode/src/pty/index.ts b/packages/opencode/src/pty/index.ts index c98e99daf..72969a555 100644 --- a/packages/opencode/src/pty/index.ts +++ b/packages/opencode/src/pty/index.ts @@ -18,27 +18,26 @@ export namespace Pty { type Socket = { readyState: number - data: object - send: (data: string | Uint8Array<ArrayBuffer> | ArrayBuffer) => void + send: (data: string | Uint8Array | ArrayBuffer) => void close: (code?: number, reason?: string) => void } - // Bun's ServerWebSocket has a per-connection `.data` object (set during - // `server.upgrade`) that changes when the underlying connection is recycled. - // We keep a reference to a stable part of it so output can't leak even when - // websocket objects are reused. - const token = (ws: Socket) => { - const data = ws.data - const events = (data as { events?: unknown }).events - if (events && typeof events === "object") return events + type Subscriber = { + id: number + } - const url = (data as { url?: unknown }).url - if (url && typeof url === "object") return url + const sockets = new WeakMap<object, number>() + const owners = new WeakMap<object, string>() + let socketCounter = 0 - return data + const tagSocket = (ws: Socket) => { + if (!ws || typeof ws !== "object") return + const next = (socketCounter = (socketCounter + 1) % Number.MAX_SAFE_INTEGER) + sockets.set(ws, next) + return next } - // WebSocket control frame: 0x00 + UTF-8 JSON (currently { cursor }). + // WebSocket control frame: 0x00 + UTF-8 JSON. const meta = (cursor: number) => { const json = JSON.stringify({ cursor }) const bytes = encoder.encode(json) @@ -102,7 +101,7 @@ export namespace Pty { buffer: string bufferCursor: number cursor: number - subscribers: Map<Socket, object> + subscribers: Map<Socket, Subscriber> } const state = Instance.state( @@ -185,13 +184,13 @@ export namespace Pty { ptyProcess.onData((chunk) => { session.cursor += chunk.length - for (const [ws, data] of session.subscribers) { + for (const [ws, sub] of session.subscribers) { if (ws.readyState !== 1) { session.subscribers.delete(ws) continue } - if (token(ws) !== data) { + if (typeof ws === "object" && sockets.get(ws) !== sub.id) { session.subscribers.delete(ws) continue } @@ -280,6 +279,25 @@ export namespace Pty { } log.info("client connected to session", { id }) + const socketId = tagSocket(ws) + if (socketId === undefined) { + ws.close() + return + } + + const previous = owners.get(ws) + if (previous && previous !== id) { + state().get(previous)?.subscribers.delete(ws) + } + + owners.set(ws, id) + session.subscribers.set(ws, { id: socketId }) + + const cleanup = () => { + session.subscribers.delete(ws) + if (owners.get(ws) === id) owners.delete(ws) + } + const start = session.bufferCursor const end = session.cursor @@ -300,6 +318,7 @@ export namespace Pty { ws.send(data.slice(i, i + BUFFER_CHUNK)) } } catch { + cleanup() ws.close() return } @@ -308,23 +327,17 @@ export namespace Pty { try { ws.send(meta(end)) } catch { + cleanup() ws.close() return } - - if (!ws.data || typeof ws.data !== "object") { - ws.close() - return - } - - session.subscribers.set(ws, token(ws)) return { onMessage: (message: string | ArrayBuffer) => { session.process.write(String(message)) }, onClose: () => { log.info("client disconnected from session", { id }) - session.subscribers.delete(ws) + cleanup() }, } } diff --git a/packages/opencode/src/server/routes/pty.ts b/packages/opencode/src/server/routes/pty.ts index d516859f7..368c9612b 100644 --- a/packages/opencode/src/server/routes/pty.ts +++ b/packages/opencode/src/server/routes/pty.ts @@ -163,18 +163,13 @@ export const PtyRoutes = lazy(() => type Socket = { readyState: number - data: object - send: (data: string | Uint8Array<ArrayBuffer> | ArrayBuffer) => void + send: (data: string | Uint8Array | ArrayBuffer) => void close: (code?: number, reason?: string) => void } const isSocket = (value: unknown): value is Socket => { if (!value || typeof value !== "object") return false if (!("readyState" in value)) return false - if (!("data" in value)) return false - if (!((value as { data?: unknown }).data && typeof (value as { data?: unknown }).data === "object")) { - return false - } if (!("send" in value) || typeof (value as { send?: unknown }).send !== "function") return false if (!("close" in value) || typeof (value as { close?: unknown }).close !== "function") return false return typeof (value as { readyState?: unknown }).readyState === "number" @@ -182,12 +177,12 @@ export const PtyRoutes = lazy(() => return { onOpen(_event, ws) { - const raw = ws.raw - if (!isSocket(raw)) { + const socket = ws.raw + if (!isSocket(socket)) { ws.close() return } - handler = Pty.connect(id, raw, cursor) + handler = Pty.connect(id, socket, cursor) }, onMessage(event) { if (typeof event.data !== "string") return diff --git a/packages/opencode/test/pty/pty-output-isolation.test.ts b/packages/opencode/test/pty/pty-output-isolation.test.ts index 337280d18..b80d37345 100644 --- a/packages/opencode/test/pty/pty-output-isolation.test.ts +++ b/packages/opencode/test/pty/pty-output-isolation.test.ts @@ -18,7 +18,6 @@ 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")) }, @@ -31,7 +30,6 @@ 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")) } @@ -53,48 +51,4 @@ 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 new onOpen handler has a chance to tag it. - 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) - } - }, - }) - }) }) diff --git a/patches/[email protected] b/patches/[email protected] deleted file mode 100644 index d63a693b8..000000000 --- a/patches/[email protected] +++ /dev/null @@ -1,40 +0,0 @@ -diff --git a/dist/ghostty-web.js b/dist/ghostty-web.js -index 7c9d64a617bbeb29d757a1acd54686e582868313..2d61098cdb77fa66cbb162897c5590f35cfcf791 100644 ---- a/dist/ghostty-web.js -+++ b/dist/ghostty-web.js -@@ -1285,7 +1285,7 @@ const e = class H { - continue; - } - const C = g.getCodepoint(); -- C === 0 || C < 32 ? B.push(" ") : B.push(String.fromCodePoint(C)); -+ C === 0 || C < 32 || C > 1114111 || (C >= 55296 && C <= 57343) ? B.push(" ") : B.push(String.fromCodePoint(C)); - } - return B.join(""); - } -@@ -1484,7 +1484,7 @@ class _ { - return; - let J = ""; - A.flags & U.ITALIC && (J += "italic "), A.flags & U.BOLD && (J += "bold "), this.ctx.font = `${J}${this.fontSize}px ${this.fontFamily}`, this.ctx.fillStyle = this.rgbToCSS(w, o, i), A.flags & U.FAINT && (this.ctx.globalAlpha = 0.5); -- const s = g, F = C + this.metrics.baseline, a = String.fromCodePoint(A.codepoint || 32); -+ const s = g, F = C + this.metrics.baseline, a = (A.codepoint === 0 || A.codepoint == null || A.codepoint < 0 || A.codepoint > 1114111 || (A.codepoint >= 55296 && A.codepoint <= 57343)) ? " " : String.fromCodePoint(A.codepoint); - if (this.ctx.fillText(a, s, F), A.flags & U.FAINT && (this.ctx.globalAlpha = 1), A.flags & U.UNDERLINE) { - const N = C + this.metrics.baseline + 2; - this.ctx.strokeStyle = this.ctx.fillStyle, this.ctx.lineWidth = 1, this.ctx.beginPath(), this.ctx.moveTo(g, N), this.ctx.lineTo(g + I, N), this.ctx.stroke(); -@@ -1730,7 +1730,7 @@ const L = class R { - let G = ""; - for (let J = M; J <= k; J++) { - const s = o[J]; -- if (s && s.codepoint !== 0) { -+ if (s && s.codepoint !== 0 && s.codepoint <= 1114111 && !(s.codepoint >= 55296 && s.codepoint <= 57343)) { - const F = String.fromCodePoint(s.codepoint); - G += F, F.trim() && (i = G.length); - } else -@@ -1995,7 +1995,7 @@ const L = class R { - if (!Q) - return null; - const g = (w) => { -- if (!w || w.codepoint === 0) -+ if (!w || w.codepoint === 0 || w.codepoint > 1114111 || (w.codepoint >= 55296 && w.codepoint <= 57343)) - return !1; - const o = String.fromCodePoint(w.codepoint); - return /[\w-]/.test(o); |