From 8a6ea61655828d92e8d2ed8706358bc5f49ff974 Mon Sep 17 00:00:00 2001 From: zahid emre zorbaz Date: Thu, 24 Sep 2026 12:20:32 +0300 Subject: [PATCH 1/2] fix(hub): kill the whole child-process tree on Windows On Windows, tinyexec runs `.cmd` shims (e.g. `node_modules/.bin/vitest.cmd`) through `cmd.exe`, so `terminate()`, `restart()` and stream cancel only killed the wrapper and left the real program running and holding its ports. Kill the tree with `taskkill /T /F` there, wait for it before `restart()` spawns the next run, and keep reporting host-initiated kills as `stopped` with an `undefined` exit code on every platform. Fixes #402 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/node/__tests__/host-terminals.test.ts | 105 +++++++++++++++++- packages/hub/src/node/host-terminals.ts | 66 +++++++++-- packages/hub/src/types/terminals.ts | 2 +- 3 files changed, 161 insertions(+), 12 deletions(-) diff --git a/packages/hub/src/node/__tests__/host-terminals.test.ts b/packages/hub/src/node/__tests__/host-terminals.test.ts index be481fec3..e14b965bd 100644 --- a/packages/hub/src/node/__tests__/host-terminals.test.ts +++ b/packages/hub/src/node/__tests__/host-terminals.test.ts @@ -1,5 +1,8 @@ import type { DevframeTerminalSession } from '../../types/terminals' import type { DevframeHubContext } from '../context' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import process from 'node:process' import { describe, expect, it, vi } from 'vitest' import { hasNative } from 'zigpty' @@ -71,8 +74,8 @@ function createTerminalHost() { } } -async function waitUntil(assertion: () => void): Promise { - const deadline = Date.now() + 1000 +async function waitUntil(assertion: () => void, timeout = 1000): Promise { + const deadline = Date.now() + timeout let lastError: unknown while (Date.now() < deadline) { try { @@ -397,6 +400,104 @@ describe('devframeTerminalHost child-process status lifecycle', () => { }) }) +// On Windows, tinyexec runs anything that isn't a `.exe`/`.com` (e.g. the +// `node_modules/.bin/*.cmd` shims package managers generate) through +// `cmd.exe /d /s /c`, so the pid the host holds belongs to `cmd.exe`, not to +// the program the shim starts. Killing only that pid leaves the real process +// orphaned (still holding its ports). +describe.runIf(process.platform === 'win32')('devframeTerminalHost child-process tree on Windows', { timeout: 20_000 }, () => { + function isAlive(pid: number): boolean { + try { + process.kill(pid, 0) + return true + } + catch { + return false + } + } + + async function startShimSession(host: DevframeTerminalsHost, id: string) { + const dir = mkdtempSync(join(tmpdir(), 'devframe-terminals-shim-')) + writeFileSync(join(dir, 'child.cjs'), 'console.log("pid:" + process.pid); setInterval(() => {}, 1000)\n') + const shim = join(dir, 'child.cmd') + writeFileSync(shim, `@"${NODE}" "%~dp0\\child.cjs" %*\r\n`) + const session = await host.startChildProcess({ command: shim, args: [], cwd: dir }, { id, title: id }) + let pid = 0 + await waitUntil(() => { + const match = session.buffer?.join('').match(/pid:(\d+)/) + expect(match).toBeTruthy() + pid = Number(match![1]) + }, 10_000) + // The host holds the `cmd.exe` wrapper, not the node child. + expect(session.getChildProcess()?.pid).not.toBe(pid) + return { session, pid, cleanup: () => rmSync(dir, { recursive: true, force: true }) } + } + + it('terminate() kills the process started by a .cmd shim', async () => { + const { host } = createTerminalHost() + const { session, pid, cleanup } = await startShimSession(host, 'shim-terminate') + try { + await session.terminate() + await waitUntil(() => expect(isAlive(pid)).toBe(false), 5000) + expect(session.status).toBe('stopped') + } + finally { + if (isAlive(pid)) + process.kill(pid) + cleanup() + } + }) + + it('terminate() reports a stopped, killed run rather than a crash', async () => { + const { host, sinks } = createTerminalHost() + const { session, pid, cleanup } = await startShimSession(host, 'shim-result') + const result = session.getResult() + try { + await session.terminate() + await waitUntil(() => expect(sinks.get('shim-result')?.closed).toBe(true)) + const output = await result + expect(output.exitCode).toBeUndefined() + expect(result.killed).toBe(true) + expect(session.status).toBe('stopped') + } + finally { + if (isAlive(pid)) + process.kill(pid) + cleanup() + } + }) + + it('restart() kills the previous run started by a .cmd shim', async () => { + const { host } = createTerminalHost() + const { session, pid, cleanup } = await startShimSession(host, 'shim-restart') + try { + await session.restart() + await waitUntil(() => expect(isAlive(pid)).toBe(false), 5000) + expect(session.status).toBe('running') + await session.terminate() + } + finally { + if (isAlive(pid)) + process.kill(pid) + cleanup() + } + }) + + it('cancelling the stream kills the process started by a .cmd shim', async () => { + const { host } = createTerminalHost() + const { session, pid, cleanup } = await startShimSession(host, 'shim-cancel') + try { + host.remove(session) + await waitUntil(() => expect(isAlive(pid)).toBe(false), 5000) + } + finally { + if (isAlive(pid)) + process.kill(pid) + cleanup() + } + }) +}) + describe('devframeTerminalHost interactive PTY sessions', () => { itPty('inherits the parent process environment', async () => { expect.assertions(1) diff --git a/packages/hub/src/node/host-terminals.ts b/packages/hub/src/node/host-terminals.ts index e98729ef2..b820ce3a3 100644 --- a/packages/hub/src/node/host-terminals.ts +++ b/packages/hub/src/node/host-terminals.ts @@ -16,6 +16,7 @@ import type { DevframeTerminalsHost as DevframeTerminalsHostType, } from '../types/terminals' import type { DevframeHubContext } from './context' +import { spawn } from 'node:child_process' import process from 'node:process' import { createEventEmitter } from 'devframe/utils/events' import { HUB_EVENTS } from '../events' @@ -35,6 +36,39 @@ const TERMINAL_BUFFER_LIMIT = 1000 /** TERM handed to spawned PTYs; also used to reject fallback process labels. */ const PTY_TERM_NAME = 'xterm-256color' +/** + * Kill a `startChildProcess()` run together with everything it spawned. + * + * On Windows, tinyexec runs anything that isn't a `.exe`/`.com` - including + * the `node_modules/.bin/*.cmd` shims package managers generate - through + * `cmd.exe /d /s /c`, so the pid held here is the wrapper's. Killing a process + * on Windows doesn't reach its descendants, so `cp.kill()` alone would leave + * the real program running (and holding its ports). `taskkill /T /F` ends the + * whole tree; `cp.kill()` stays the fallback and the POSIX path. + */ +function killProcessTree(cp: TinyExecResult): Promise { + const child = cp.process + const pid = child?.pid + if (process.platform !== 'win32' || !child || pid === undefined || child.exitCode !== null || child.signalCode !== null) { + cp.kill() + return Promise.resolve() + } + return new Promise((resolve) => { + let finished = false + const finish = (ok: boolean) => { + if (finished) + return + finished = true + if (!ok) + cp.kill() + resolve() + } + const killer = spawn('taskkill', ['/pid', String(pid), '/T', '/F'], { stdio: 'ignore', windowsHide: true }) + killer.once('error', () => finish(false)) + killer.once('exit', code => finish(code === 0)) + }) +} + export class DevframeTerminalsHost implements DevframeTerminalsHostType { public readonly sessions: DevframeTerminalsHostType['sessions'] = new Map() public readonly events: DevframeTerminalsHostType['events'] = createEventEmitter() @@ -231,15 +265,26 @@ export class DevframeTerminalsHost implements DevframeTerminalsHostType { let cp: TinyExecResult | undefined let currentResult: DevframeChildProcessResult | undefined let runId = 0 + // Runs stopped on purpose (terminate/restart/cancel). On Windows the tree + // kill ends them with exit code 1 rather than a signal, so this keeps them + // reported as a deliberate stop instead of a crash on every platform. + const hostKilled = new WeakSet() + const killRun = (target: TinyExecResult | undefined): Promise => { + if (!target) + return Promise.resolve() + hostKilled.add(target) + return killProcessTree(target) + } const stream = new ReadableStream({ start(_controller) { state.controller = _controller }, cancel() { - cp?.kill() + const target = cp cp = undefined closeStream() + return killRun(target) }, }) @@ -310,15 +355,16 @@ export class DevframeTerminalsHost implements DevframeTerminalsHostType { markStatus('error') }) cp.process?.once('close', (code) => { - settle(code ?? undefined) + const killed = hostKilled.has(cp) + settle(killed ? undefined : code ?? undefined) if (currentRun !== runId) return closeStream() // A spawn/runtime error already settled the status; a non-zero exit - // code is a crash. A clean exit, or a signal kill (no numeric code, - // e.g. terminate()/restart()), is a deliberate/normal stop. + // code is a crash. A clean exit, or a kill by the host (terminate()/ + // restart()), is a deliberate/normal stop. if (!runErrored) - markStatus(typeof code === 'number' && code !== 0 ? 'error' : 'stopped') + markStatus(!killed && typeof code === 'number' && code !== 0 ? 'error' : 'stopped') }) currentResult = { @@ -326,10 +372,10 @@ export class DevframeTerminalsHost implements DevframeTerminalsHostType { return cp.process?.pid }, get exitCode() { - return cp.process?.exitCode ?? undefined + return hostKilled.has(cp) ? undefined : cp.process?.exitCode ?? undefined }, get killed() { - return cp.process?.killed === true + return hostKilled.has(cp) || cp.process?.killed === true }, kill: signal => cp.kill(signal), then: (onfulfilled, onrejected) => outputPromise.then(onfulfilled, onrejected), @@ -343,13 +389,15 @@ export class DevframeTerminalsHost implements DevframeTerminalsHostType { const restart = async () => { if (state.streamClosed) throw diagnostics.DF8206({ id: terminal.id }) - cp?.kill() + // Wait for the old tree to go away so the new run can reclaim its ports. + await killRun(cp) cp = createChildProcess() markStatus('running') } const terminate = async () => { - cp?.kill() + const target = cp cp = undefined + await killRun(target) closeStream() markStatus('stopped') } diff --git a/packages/hub/src/types/terminals.ts b/packages/hub/src/types/terminals.ts index 0f1fcdd81..feced0e46 100644 --- a/packages/hub/src/types/terminals.ts +++ b/packages/hub/src/types/terminals.ts @@ -79,7 +79,7 @@ export interface DevframeChildProcessExecuteOptions { * The settled outcome of a {@link DevframeChildProcessTerminalSession} run: * stdout/stderr captured separately (unlike the session's merged display * `stream`), plus the process's exit code (`undefined` if it was killed by a - * signal before exiting). + * signal, or by `terminate()`/`restart()`, before exiting). */ export interface DevframeChildProcessOutput { stdout: string From dbc84499da4329d5dd042ce7be2f0c6d786502c3 Mon Sep 17 00:00:00 2001 From: "Anthony Fu (via agent)" Date: Fri, 25 Sep 2026 00:45:27 +0000 Subject: [PATCH 2/2] refactor(hub): run taskkill through tinyexec --- packages/hub/src/node/host-terminals.ts | 27 +++++++++---------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/packages/hub/src/node/host-terminals.ts b/packages/hub/src/node/host-terminals.ts index b820ce3a3..58b25ba52 100644 --- a/packages/hub/src/node/host-terminals.ts +++ b/packages/hub/src/node/host-terminals.ts @@ -16,7 +16,6 @@ import type { DevframeTerminalsHost as DevframeTerminalsHostType, } from '../types/terminals' import type { DevframeHubContext } from './context' -import { spawn } from 'node:child_process' import process from 'node:process' import { createEventEmitter } from 'devframe/utils/events' import { HUB_EVENTS } from '../events' @@ -46,27 +45,21 @@ const PTY_TERM_NAME = 'xterm-256color' * the real program running (and holding its ports). `taskkill /T /F` ends the * whole tree; `cp.kill()` stays the fallback and the POSIX path. */ -function killProcessTree(cp: TinyExecResult): Promise { +async function killProcessTree(cp: TinyExecResult): Promise { const child = cp.process const pid = child?.pid if (process.platform !== 'win32' || !child || pid === undefined || child.exitCode !== null || child.signalCode !== null) { cp.kill() - return Promise.resolve() + return } - return new Promise((resolve) => { - let finished = false - const finish = (ok: boolean) => { - if (finished) - return - finished = true - if (!ok) - cp.kill() - resolve() - } - const killer = spawn('taskkill', ['/pid', String(pid), '/T', '/F'], { stdio: 'ignore', windowsHide: true }) - killer.once('error', () => finish(false)) - killer.once('exit', code => finish(code === 0)) - }) + const { exec } = await import('tinyexec') + try { + const { exitCode } = await exec('taskkill', ['/pid', String(pid), '/T', '/F']) + if (exitCode === 0) + return + } + catch {} + cp.kill() } export class DevframeTerminalsHost implements DevframeTerminalsHostType {