Skip to content

Commit 5a50817

Browse files
committed
fix(hub): bound terminal scrollback buffer and reject restart after terminate
Caps the per-session terminal scrollback buffer at 1000 chunks (mirroring the streaming replay window) instead of growing unbounded for the lifetime of a long-running PTY/child-process session. Also makes both restart() closures no-op once the session's stream has been terminated, so restarting a terminated session no longer spawns an orphaned, unmanaged process.
1 parent 48aa791 commit 5a50817

4 files changed

Lines changed: 46 additions & 209 deletions

File tree

packages/hub/src/node/__tests__/host-terminals.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,42 @@ describe('devframeTerminalHost stream lifecycle', () => {
141141
})
142142
expect(session.getChildProcess()).toBeUndefined()
143143
})
144+
145+
it('bounds the session scrollback buffer', async () => {
146+
const { host } = createTerminalHost()
147+
let controller: ReadableStreamDefaultController<string>
148+
const stream = new ReadableStream<string>({
149+
start(_controller) {
150+
controller = _controller
151+
},
152+
})
153+
const session: DevframeTerminalSession = { id: 't', title: 'T', status: 'running', stream }
154+
host.register(session)
155+
for (let i = 0; i < 1200; i++) controller!.enqueue(`line-${i}`)
156+
// Wait for the read loop to drain every enqueued chunk (each read is its
157+
// own microtask tick) before asserting the trimmed shape.
158+
await waitUntil(() => {
159+
expect(session.buffer!.at(-1)).toBe('line-1199')
160+
})
161+
// Keeps the newest, drops the oldest.
162+
expect(session.buffer!.length).toBeLessThanOrEqual(1000)
163+
expect(session.buffer!.includes('line-0')).toBe(false)
164+
})
165+
166+
it('does not restart a terminated child-process session', async () => {
167+
const { host, sinks } = createTerminalHost()
168+
const session = await host.startChildProcess(
169+
{ command: process.execPath, args: ['-e', 'setInterval(() => {}, 1000)'] },
170+
{ id: 'child', title: 'Child' },
171+
)
172+
await session.terminate()
173+
await waitUntil(() => {
174+
expect(sinks.get('child')?.closed).toBe(true)
175+
})
176+
await session.restart()
177+
// Stream stays closed; no orphan output stream.
178+
expect(sinks.get('child')?.closed).toBe(true)
179+
})
144180
})
145181

146182
describe('devframeTerminalHost interactive PTY sessions', () => {

packages/hub/src/node/host-terminals.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ type PartialWithoutId<T extends { id: string }> = Partial<T> & { id: string }
2323
*/
2424
const TERMINAL_STREAM_CHANNEL = 'devframe:terminals' as const
2525
const TERMINAL_REPLAY_WINDOW = 1000
26+
/** Max chunks retained in the per-session scrollback buffer (bounded like the replay window). */
27+
const TERMINAL_BUFFER_LIMIT = 1000
2628

2729
/** TERM handed to spawned PTYs; also used to reject fallback process labels. */
2830
const PTY_TERM_NAME = 'xterm-256color'
@@ -123,8 +125,10 @@ export class DevframeTerminalsHost implements DevframeTerminalsHostType {
123125
if (result.done)
124126
break
125127
// Mirror to the legacy session.buffer used by `terminals:read` —
126-
// unbounded history kept for the snapshot endpoint.
128+
// bounded tail kept for the snapshot endpoint.
127129
sessionBuffer.push(result.value)
130+
if (sessionBuffer.length > TERMINAL_BUFFER_LIMIT)
131+
sessionBuffer.splice(0, sessionBuffer.length - TERMINAL_BUFFER_LIMIT)
128132
sink?.write(result.value)
129133
}
130134
if (!disposed && sink && !sink.closed)
@@ -243,6 +247,8 @@ export class DevframeTerminalsHost implements DevframeTerminalsHostType {
243247
cp = createChildProcess()
244248

245249
const restart = async () => {
250+
if (streamClosed)
251+
return
246252
cp?.kill()
247253
cp = createChildProcess()
248254
}
@@ -394,6 +400,8 @@ export class DevframeTerminalsHost implements DevframeTerminalsHostType {
394400
closeStream()
395401
},
396402
restart: async () => {
403+
if (streamClosed)
404+
return
397405
pty?.kill()
398406
pty = spawnPty()
399407
},

plans/008-cap-terminal-buffer.md

Lines changed: 0 additions & 207 deletions
This file was deleted.

plans/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ changes are allowed as long as they're marked).
2020
| 005 | Don't cache a rejected RPC `setup()` promise | bug | P1 | S || DONE |
2121
| 006 | Bound `SharedState.syncIds` (memory leak) | bug | P1 | S || DONE |
2222
| 007 | Behavioral tests for the auth/OTP trust boundary | tests | P1 | S || DONE |
23-
| 008 | Cap hub terminal buffer + reject restart-after-terminate | bug | P2 | S || TODO |
23+
| 008 | Cap hub terminal buffer + reject restart-after-terminate | bug | P2 | S || DONE |
2424
| 009 | Fix two server-side streaming lifecycle leaks | bug | P2 | S-M || TODO |
2525
| 010 | Initialize client shared state once across trust flips | bug | P2 | S || TODO |
2626
| 011 | Don't leak/drop when WS client posts on a closing socket | bug | P2 | S || DONE |

0 commit comments

Comments
 (0)