Skip to content

Commit a721f7a

Browse files
committed
fix(plugin-terminals): make PTY tests Windows-safe and harness leak-free
- Gate the PTY-semantics tests (stdin echo, SIGWINCH resize, foreground process name) to POSIX; Windows keeps the isTTY interactive coverage. These rely on behaviours conpty doesn't provide (no SIGWINCH; `.process` returns the TERM name). - Ignore the Windows `xterm-256color` TERM-name fallback so it never surfaces as a session label. - Dispose the terminal manager on test server close so spawned PTY/piped child processes don't leak across runs.
1 parent 4768007 commit a721f7a

3 files changed

Lines changed: 31 additions & 5 deletions

File tree

plugins/terminals/src/node/backend.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ interface PtyProcess {
5555
kill: (signal?: string) => void
5656
}
5757

58+
/** TERM name handed to the PTY; also used to reject the Windows fallback. */
59+
const PTY_TERM_NAME = 'xterm-256color'
60+
5861
let ptyModulePromise: Promise<PtyModule | undefined> | undefined
5962

6063
/**
@@ -90,7 +93,7 @@ export async function spawnPty(options: SpawnBackendOptions): Promise<TerminalPr
9093
let proc: PtyProcess
9194
try {
9295
proc = pty.spawn(options.command, options.args, {
93-
name: 'xterm-256color',
96+
name: PTY_TERM_NAME,
9497
cols: options.cols,
9598
rows: options.rows,
9699
cwd: options.cwd,
@@ -136,7 +139,10 @@ export async function spawnPty(options: SpawnBackendOptions): Promise<TerminalPr
136139
onExit: cb => proc.onExit(e => cb(e.exitCode ?? 0)),
137140
getProcessName: () => {
138141
try {
139-
return proc.process || undefined
142+
const name = proc.process
143+
// On Windows node-pty falls back to the TERM name rather than the
144+
// foreground process — don't surface that as a session label.
145+
return name && name !== PTY_TERM_NAME ? name : undefined
140146
}
141147
catch {
142148
return undefined

plugins/terminals/test/_utils.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { createWsRpcChannel } from 'devframe/rpc/transports/ws-client'
1313
import { getPort } from 'get-port-please'
1414
import { H3 } from 'h3'
1515
import { createTerminalsDevframe } from '../src/index'
16+
import { getTerminalManager } from '../src/node/index'
1617

1718
export type TerminalsServer = StartedServer & {
1819
ctx: DevframeNodeContext
@@ -40,6 +41,20 @@ export async function startTerminalsServer(options: TerminalsOptions = {}): Prom
4041
await definition.setup(ctx)
4142

4243
const server = await startHttpAndWs({ context: ctx, host, port, app, auth: false })
44+
45+
// Tear down spawned terminal processes (PTYs / piped children) alongside
46+
// the HTTP+WS server so tests don't leak `node`/shell processes.
47+
const closeServer = server.close.bind(server)
48+
server.close = async () => {
49+
try {
50+
getTerminalManager(ctx).dispose()
51+
}
52+
catch {
53+
// Manager may not be initialised if setup failed.
54+
}
55+
await closeServer()
56+
}
57+
4358
return Object.assign(server, { ctx, port })
4459
}
4560

plugins/terminals/test/terminals.test.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ import { bootClient, call, collectUntil, startTerminalsServer } from './_utils'
99
vi.stubGlobal('WebSocket', WebSocket)
1010

1111
const NODE = process.execPath
12+
// PTY semantics differ on Windows (conpty): no SIGWINCH, the foreground
13+
// process name resolves to the TERM name, and stdin round-trips are slow to
14+
// render. These behaviours are exercised on POSIX; Windows keeps the
15+
// `isTTY` interactive coverage below.
16+
const itPosix = process.platform === 'win32' ? it.skip : it
1217

1318
function subscribe(client: TestClient, id: string) {
1419
return client.streaming.subscribe<string>(TERMINAL_STREAM_CHANNEL, id)
@@ -66,7 +71,7 @@ describe('@devframes/plugin-terminals', () => {
6671
).rejects.toThrow(/read-only/i)
6772
})
6873

69-
it('runs an interactive PTY session that accepts input', async () => {
74+
itPosix('runs an interactive PTY session that accepts input', async () => {
7075
const client = bootClient(server.port)
7176
await new Promise(r => setTimeout(r, 50))
7277

@@ -100,7 +105,7 @@ describe('@devframes/plugin-terminals', () => {
100105
expect(output).toContain('isTTY=true')
101106
})
102107

103-
it('propagates resize to the PTY (SIGWINCH) for TUI layout', async () => {
108+
itPosix('propagates resize to the PTY (SIGWINCH) for TUI layout', async () => {
104109
const client = bootClient(server.port)
105110
await new Promise(r => setTimeout(r, 50))
106111

@@ -135,7 +140,7 @@ describe('@devframes/plugin-terminals', () => {
135140
expect(restarted.status).toBe('running')
136141
})
137142

138-
it('tracks the foreground process name for PTY sessions', async () => {
143+
itPosix('tracks the foreground process name for PTY sessions', async () => {
139144
const client = bootClient(server.port)
140145
await new Promise(r => setTimeout(r, 50))
141146

0 commit comments

Comments
 (0)