Skip to content

Commit 4e9ef3e

Browse files
authored
fix(app): terminal issues (#14435)
1 parent 7e0e35a commit 4e9ef3e

5 files changed

Lines changed: 91 additions & 24 deletions

File tree

packages/app/e2e/terminal/terminal-init.spec.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ test("smoke terminal mounts and can create a second tab", async ({ page, gotoSes
66
await gotoSession()
77

88
const terminals = page.locator(terminalSelector)
9+
const tabs = page.locator('#terminal-panel [data-slot="tabs-trigger"]')
910
const opened = await terminals.first().isVisible()
1011

1112
if (!opened) {
@@ -21,6 +22,7 @@ test("smoke terminal mounts and can create a second tab", async ({ page, gotoSes
2122
await page.locator(promptSelector).click()
2223
await page.keyboard.press("Control+Alt+T")
2324

24-
await expect(terminals).toHaveCount(2)
25-
await expect(terminals.nth(1).locator("textarea")).toHaveCount(1)
25+
await expect(tabs).toHaveCount(2)
26+
await expect(terminals).toHaveCount(1)
27+
await expect(terminals.first().locator("textarea")).toHaveCount(1)
2628
})

packages/app/src/components/terminal.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ export const Terminal = (props: TerminalProps) => {
540540
disposed = true
541541
if (fitFrame !== undefined) cancelAnimationFrame(fitFrame)
542542
if (sizeTimer !== undefined) clearTimeout(sizeTimer)
543-
if (ws && ws.readyState !== WebSocket.CLOSED && ws.readyState !== WebSocket.CLOSING) ws.close()
543+
if (ws && ws.readyState !== WebSocket.CLOSED && ws.readyState !== WebSocket.CLOSING) ws.close(1000)
544544

545545
const finalize = () => {
546546
persistTerminal({ term, addon: serializeAddon, cursor, pty: local.pty, onCleanup: props.onCleanup })

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

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,11 @@ export function TerminalPanel() {
6767
on(
6868
() => terminal.active(),
6969
(activeId) => {
70-
if (!activeId || !opened()) return
70+
if (!activeId || !open()) return
7171
if (document.activeElement instanceof HTMLElement) {
7272
document.activeElement.blur()
7373
}
74-
focusTerminalById(activeId)
74+
setTimeout(() => focusTerminalById(activeId), 0)
7575
},
7676
),
7777
)
@@ -209,21 +209,17 @@ export function TerminalPanel() {
209209
</Tabs.List>
210210
</Tabs>
211211
<div class="flex-1 min-h-0 relative">
212-
<For each={all()}>
213-
{(pty) => (
214-
<div
215-
id={`terminal-wrapper-${pty.id}`}
216-
class="absolute inset-0"
217-
style={{
218-
display: terminal.active() === pty.id ? "block" : "none",
219-
}}
220-
>
221-
<Show when={pty.id} keyed>
222-
<Terminal pty={pty} onCleanup={terminal.update} onConnectError={() => terminal.clone(pty.id)} />
223-
</Show>
224-
</div>
212+
<Show when={terminal.active()} keyed>
213+
{(id) => (
214+
<Show when={byId().get(id)}>
215+
{(pty) => (
216+
<div id={`terminal-wrapper-${id}`} class="absolute inset-0">
217+
<Terminal pty={pty()} onCleanup={terminal.update} onConnectError={() => terminal.clone(id)} />
218+
</div>
219+
)}
220+
</Show>
225221
)}
226-
</For>
222+
</Show>
227223
</div>
228224
</div>
229225
<DragOverlay>

packages/opencode/src/pty/index.ts

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,38 @@ export namespace Pty {
4141

4242
const token = (ws: Socket) => {
4343
const data = ws.data
44-
if (!data || typeof data !== "object") return
44+
if (data === undefined) return
45+
if (data === null) return
46+
if (typeof data !== "object") return data
4547

46-
const events = (data as { events?: unknown }).events
47-
if (events && typeof events === "object") return events
48+
const id = (data as { connId?: unknown }).connId
49+
if (typeof id === "number" || typeof id === "string") return id
50+
51+
const href = (data as { href?: unknown }).href
52+
if (typeof href === "string") return href
4853

4954
const url = (data as { url?: unknown }).url
50-
if (url && typeof url === "object") return url
55+
if (typeof url === "string") return url
56+
if (url && typeof url === "object") {
57+
const href = (url as { href?: unknown }).href
58+
if (typeof href === "string") return href
59+
return url
60+
}
61+
62+
const events = (data as { events?: unknown }).events
63+
if (typeof events === "number" || typeof events === "string") return events
64+
if (events && typeof events === "object") {
65+
const id = (events as { connId?: unknown }).connId
66+
if (typeof id === "number" || typeof id === "string") return id
67+
68+
const id2 = (events as { connection?: unknown }).connection
69+
if (typeof id2 === "number" || typeof id2 === "string") return id2
70+
71+
const id3 = (events as { id?: unknown }).id
72+
if (typeof id3 === "number" || typeof id3 === "string") return id3
73+
74+
return events
75+
}
5176

5277
return data
5378
}
@@ -210,7 +235,7 @@ export namespace Pty {
210235
continue
211236
}
212237

213-
if (sub.token !== undefined && token(ws) !== sub.token) {
238+
if (token(ws) !== sub.token) {
214239
session.subscribers.delete(ws)
215240
continue
216241
}

packages/opencode/test/pty/pty-output-isolation.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,48 @@ describe("pty", () => {
9797
},
9898
})
9999
})
100+
101+
test("does not leak output when socket data mutates in-place", async () => {
102+
await using dir = await tmpdir({ git: true })
103+
104+
await Instance.provide({
105+
directory: dir.path,
106+
fn: async () => {
107+
const a = await Pty.create({ command: "cat", title: "a" })
108+
try {
109+
const outA: string[] = []
110+
const outB: string[] = []
111+
112+
const ctx = { connId: 1 }
113+
const ws = {
114+
readyState: 1,
115+
data: ctx,
116+
send: (data: unknown) => {
117+
outA.push(typeof data === "string" ? data : Buffer.from(data as Uint8Array).toString("utf8"))
118+
},
119+
close: () => {
120+
// no-op
121+
},
122+
}
123+
124+
Pty.connect(a.id, ws as any)
125+
outA.length = 0
126+
127+
// Simulate the runtime mutating per-connection data without
128+
// swapping the reference (ws.data stays the same object).
129+
ctx.connId = 2
130+
ws.send = (data: unknown) => {
131+
outB.push(typeof data === "string" ? data : Buffer.from(data as Uint8Array).toString("utf8"))
132+
}
133+
134+
Pty.write(a.id, "AAA\n")
135+
await Bun.sleep(100)
136+
137+
expect(outB.join("")).not.toContain("AAA")
138+
} finally {
139+
await Pty.remove(a.id)
140+
}
141+
},
142+
})
143+
})
100144
})

0 commit comments

Comments
 (0)