From 2cef80a80bb32481947754d63018df3c4ea7b446 Mon Sep 17 00:00:00 2001 From: Julien Huang Date: Mon, 26 Oct 2026 23:50:33 +0100 Subject: [PATCH 1/2] fix(hub): run `.cmd`/`.bat` shims in PTY sessions on Windows --- .../src/node/__tests__/host-terminals.test.ts | 24 ++++++ .../src/node/__tests__/pty-command.test.ts | 65 ++++++++++++++ packages/hub/src/node/host-terminals.ts | 22 +++-- packages/hub/src/node/pty-command.ts | 84 +++++++++++++++++++ 4 files changed, 186 insertions(+), 9 deletions(-) create mode 100644 packages/hub/src/node/__tests__/pty-command.test.ts create mode 100644 packages/hub/src/node/pty-command.ts diff --git a/packages/hub/src/node/__tests__/host-terminals.test.ts b/packages/hub/src/node/__tests__/host-terminals.test.ts index 18434d83..c8e29a0b 100644 --- a/packages/hub/src/node/__tests__/host-terminals.test.ts +++ b/packages/hub/src/node/__tests__/host-terminals.test.ts @@ -31,6 +31,7 @@ const itPty = hasNative ? it : it.skip // doesn't reliably provide (echo timing); skip there, mirroring the terminals // plugin's own `itPosixPty` gate. const itPosixPty = (hasNative && process.platform !== 'win32') ? it : it.skip +const itWindowsPty = (hasNative && process.platform === 'win32') ? it : it.skip interface FakeSink { write: ReturnType @@ -613,6 +614,29 @@ describe('devframeTerminalHost interactive PTY sessions', () => { expect(result.killed).toBe(false) }) + itWindowsPty('runs .cmd shims, which ConPTY cannot spawn directly', async () => { + const dir = mkdtempSync(join(tmpdir(), 'devframe-pty-shim-')) + try { + const pathKey = Object.keys(process.env).find(key => key.toUpperCase() === 'PATH') ?? 'PATH' + writeFileSync(join(dir, 'echo-args.cmd'), `@"${NODE}" -e "process.stdout.write(JSON.stringify(process.argv.slice(1)))" %*\r\n`) + const { host } = createTerminalHost() + + const session = await host.startPtySession({ + command: 'echo-args', + args: ['a&b', 'c d'], + // Reuse the inherited key: Windows env blocks are case-insensitive. + env: { [pathKey]: `${dir};${process.env[pathKey]}` }, + }, { id: 'pty-cmd-shim', title: 'PTY .cmd shim' }) + const output = await session.getResult() + + expect(output.output).toContain('["a&b","c d"]') + expect(output.exitCode).toBe(0) + } + finally { + rmSync(dir, { recursive: true, force: true }) + } + }) + itPty('getResult() preserves a non-zero PTY exit code', async () => { expect.assertions(3) diff --git a/packages/hub/src/node/__tests__/pty-command.test.ts b/packages/hub/src/node/__tests__/pty-command.test.ts new file mode 100644 index 00000000..78563d2c --- /dev/null +++ b/packages/hub/src/node/__tests__/pty-command.test.ts @@ -0,0 +1,65 @@ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterAll, describe, expect, it } from 'vitest' +import { resolvePtyCommand } from '../pty-command' + +const bin = mkdtempSync(join(tmpdir(), 'devframe-pty-command-')) +writeFileSync(join(bin, 'pnpm.cmd'), '') +writeFileSync(join(bin, 'legacy.bat'), '') +writeFileSync(join(bin, 'node.exe'), '') + +afterAll(() => rmSync(bin, { recursive: true, force: true })) + +const win32 = { cwd: tmpdir(), platform: 'win32' as const, env: { Path: bin, ComSpec: 'C:\\Windows\\system32\\cmd.exe' } } + +describe('resolvePtyCommand', () => { + it('runs .cmd shims through ComSpec on win32', () => { + expect(resolvePtyCommand('pnpm', ['exec', 'storybook', 'dev', '-p', '6006'], win32)).toEqual({ + command: 'C:\\Windows\\system32\\cmd.exe', + args: ['/d', '/c', 'pnpm', 'exec', 'storybook', 'dev', '-p', '6006'], + }) + }) + + it('runs .bat files through ComSpec on win32', () => { + expect(resolvePtyCommand('legacy', [], win32).command).toBe('C:\\Windows\\system32\\cmd.exe') + }) + + it('falls back to cmd.exe without ComSpec', () => { + expect(resolvePtyCommand('pnpm', [], { ...win32, env: { PATH: bin } }).command).toBe('cmd.exe') + }) + + it('runs unresolved commands through cmd.exe so a typo fails like a shell would', () => { + expect(resolvePtyCommand('does-not-exist', ['x'], win32).args).toEqual(['/d', '/c', 'does-not-exist', 'x']) + }) + + it('spawns real executables directly on win32', () => { + expect(resolvePtyCommand('node', ['-v'], win32)).toEqual({ command: 'node', args: ['-v'] }) + expect(resolvePtyCommand(join(bin, 'node.exe'), ['-v'], win32)).toEqual({ command: join(bin, 'node.exe'), args: ['-v'] }) + }) + + it('does not wrap cmd.exe itself', () => { + expect(resolvePtyCommand('cmd.exe', ['/c', 'dir'], win32)).toEqual({ command: 'cmd.exe', args: ['/c', 'dir'] }) + expect(resolvePtyCommand('C:\\Windows\\system32\\cmd.exe', [], win32).args).toEqual([]) + }) + + it('escapes cmd metacharacters in bare arguments, twice for batch shims', () => { + expect(resolvePtyCommand('does-not-exist', ['a&b', 'x(y)'], win32).args).toEqual(['/d', '/c', 'does-not-exist', 'a^&b', 'x^(y^)']) + expect(resolvePtyCommand('pnpm', ['a&b', '50%'], win32).args).toEqual(['/d', '/c', 'pnpm', 'a^^^&b', '50^^^%']) + }) + + it('leaves arguments zigpty quotes untouched', () => { + expect(resolvePtyCommand('pnpm', ['a b&c', '', 'say "hi"'], win32).args).toEqual(['/d', '/c', 'pnpm', 'a b&c', '', 'say "hi"']) + }) + + it('skips the cwd when NoDefaultCurrentDirectoryInExePath is set', () => { + const env = { NoDefaultCurrentDirectoryInExePath: '1', ComSpec: 'cmd.exe' } + expect(resolvePtyCommand('node', [], { ...win32, cwd: bin, env }).command).toBe('cmd.exe') + expect(resolvePtyCommand('node', [], { ...win32, cwd: bin, env: { ComSpec: 'cmd.exe' } }).command).toBe('node') + }) + + it('leaves the command unchanged on other platforms', () => { + for (const platform of ['linux', 'darwin'] as const) + expect(resolvePtyCommand('pnpm', ['a&b'], { ...win32, platform })).toEqual({ command: 'pnpm', args: ['a&b'] }) + }) +}) diff --git a/packages/hub/src/node/host-terminals.ts b/packages/hub/src/node/host-terminals.ts index 58b25ba5..d14df7b2 100644 --- a/packages/hub/src/node/host-terminals.ts +++ b/packages/hub/src/node/host-terminals.ts @@ -20,6 +20,7 @@ import process from 'node:process' import { createEventEmitter } from 'devframe/utils/events' import { HUB_EVENTS } from '../events' import { diagnostics } from './diagnostics' +import { resolvePtyCommand } from './pty-command' type PartialWithoutId = Partial & { id: string } @@ -443,18 +444,21 @@ export class DevframeTerminalsHost implements DevframeTerminalsHostType { const spawnPty = (): IPty => { const currentRun = ++runId - const ptyProcess = spawn(executeOptions.command, executeOptions.args ?? [], { + const cwd = executeOptions.cwd ?? process.cwd() + const env = { + ...process.env, + TERM: PTY_TERM_NAME, + COLORTERM: 'truecolor', + FORCE_COLOR: '1', + ...(executeOptions.env ?? {}), + } + const { command, args } = resolvePtyCommand(executeOptions.command, executeOptions.args ?? [], { cwd, env }) + const ptyProcess = spawn(command, args, { name: PTY_TERM_NAME, cols, rows, - cwd: executeOptions.cwd ?? process.cwd(), - env: { - ...process.env, - TERM: PTY_TERM_NAME, - COLORTERM: 'truecolor', - FORCE_COLOR: '1', - ...(executeOptions.env ?? {}), - }, + cwd, + env, }) const outputChunks: string[] = [] let killed = false diff --git a/packages/hub/src/node/pty-command.ts b/packages/hub/src/node/pty-command.ts new file mode 100644 index 00000000..aec340fd --- /dev/null +++ b/packages/hub/src/node/pty-command.ts @@ -0,0 +1,84 @@ +import { existsSync } from 'node:fs' +import { extname, resolve } from 'node:path' +import process from 'node:process' + +export interface PtyCommand { + command: string + args: string[] +} + +export interface ResolvePtyCommandOptions { + cwd: string + env: Record + platform?: NodeJS.Platform +} + +const DEFAULT_PATHEXT = '.COM;.EXE;.BAT;.CMD' +// Same set cross-spawn escapes for `cmd.exe`. +const CMD_META_CHARS = /[()[\]%!^"`<>&|;, *?]/g + +export function resolvePtyCommand( + command: string, + args: string[], + { cwd, env, platform = process.platform }: ResolvePtyCommandOptions, +): PtyCommand { + if (platform !== 'win32') + return { command, args } + + const comSpec = getEnv(env, 'ComSpec') || 'cmd.exe' + if (isCmd(command, comSpec)) + return { command, args } + + const resolved = findExecutable(command, cwd, env) + const ext = resolved ? extname(resolved).toLowerCase() : '' + if (ext === '.exe' || ext === '.com') + return { command, args } + + // `.cmd`/`.bat` shims re-parse their arguments + // metachar need a second round of escaping to survive both passes + const isBatch = ext === '.cmd' || ext === '.bat' + return { + command: comSpec, + args: ['/d', '/c', escapeCmd(command, false), ...args.map(arg => escapeCmd(arg, isBatch))], + } +} + +function getEnv(env: ResolvePtyCommandOptions['env'], name: string): string | undefined { + const key = Object.keys(env).findLast(key => key.toLowerCase() === name.toLowerCase()) + return key ? env[key] : undefined +} + +function isCmd(command: string, comSpec: string): boolean { + const name = command.toLowerCase() + return name === comSpec.toLowerCase() || name === 'cmd' || name === 'cmd.exe' +} + +function findExecutable(command: string, cwd: string, env: ResolvePtyCommandOptions['env']): string | undefined { + const extensions = (getEnv(env, 'PATHEXT') || DEFAULT_PATHEXT).split(';').filter(Boolean) + const candidates = extname(command) ? [command, ...extensions.map(ext => command + ext)] : extensions.map(ext => command + ext) + + const searchCwd = getEnv(env, 'NoDefaultCurrentDirectoryInExePath') === undefined + const dirs = /[\\/]/.test(command) + ? [cwd] + : [...(searchCwd ? [cwd] : []), ...(getEnv(env, 'PATH') || '').split(';').filter(Boolean)] + for (const dir of dirs) { + for (const candidate of candidates) { + const file = resolve(dir, candidate) + if (existsSync(file)) + return file + } + } + return undefined +} + +/** + * Escape `cmd.exe` metacharacters in an argument zigpty passes unquoted. + * zigpty already double-quotes arguments with whitespace or quotes, and + * `cmd.exe` keeps quoted metacharacters literal, so those are left alone. + */ +function escapeCmd(arg: string, doubleEscape: boolean): string { + if (!arg || /[\s"]/.test(arg)) + return arg + const escaped = arg.replace(CMD_META_CHARS, '^$&') + return doubleEscape ? escaped.replace(CMD_META_CHARS, '^$&') : escaped +} From 6ed7cfe7bf871b343077b6ca02702d21d9c23d7c Mon Sep 17 00:00:00 2001 From: "Anthony Fu (via agent)" Date: Fri, 30 Oct 2026 22:09:59 +0000 Subject: [PATCH 2/2] fix(hub): match zigpty quoting when escaping PTY shim args - Skip `^` escaping for args with a backslash or tab too, since zigpty quotes those and `cmd.exe` keeps quoted text as-is. - Lower-case PATHEXT so the lookup works on case-sensitive filesystems (the unit tests failed on Linux). - Note the known limit for shims passed by path with quoted args. Co-Authored-By: Claude Opus 5.5 --- .../hub/src/node/__tests__/host-terminals.test.ts | 4 ++-- packages/hub/src/node/__tests__/pty-command.test.ts | 7 ++++++- packages/hub/src/node/pty-command.ts | 12 ++++++++---- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/packages/hub/src/node/__tests__/host-terminals.test.ts b/packages/hub/src/node/__tests__/host-terminals.test.ts index c8e29a0b..70efadda 100644 --- a/packages/hub/src/node/__tests__/host-terminals.test.ts +++ b/packages/hub/src/node/__tests__/host-terminals.test.ts @@ -623,13 +623,13 @@ describe('devframeTerminalHost interactive PTY sessions', () => { const session = await host.startPtySession({ command: 'echo-args', - args: ['a&b', 'c d'], + args: ['a&b', 'c d', 'src\\x&y'], // Reuse the inherited key: Windows env blocks are case-insensitive. env: { [pathKey]: `${dir};${process.env[pathKey]}` }, }, { id: 'pty-cmd-shim', title: 'PTY .cmd shim' }) const output = await session.getResult() - expect(output.output).toContain('["a&b","c d"]') + expect(output.output).toContain('["a&b","c d","src\\\\x&y"]') expect(output.exitCode).toBe(0) } finally { diff --git a/packages/hub/src/node/__tests__/pty-command.test.ts b/packages/hub/src/node/__tests__/pty-command.test.ts index 78563d2c..eae0faa1 100644 --- a/packages/hub/src/node/__tests__/pty-command.test.ts +++ b/packages/hub/src/node/__tests__/pty-command.test.ts @@ -49,7 +49,12 @@ describe('resolvePtyCommand', () => { }) it('leaves arguments zigpty quotes untouched', () => { - expect(resolvePtyCommand('pnpm', ['a b&c', '', 'say "hi"'], win32).args).toEqual(['/d', '/c', 'pnpm', 'a b&c', '', 'say "hi"']) + expect(resolvePtyCommand('pnpm', ['a b&c', '', 'say "hi"', 'a\tb&c'], win32).args).toEqual(['/d', '/c', 'pnpm', 'a b&c', '', 'say "hi"', 'a\tb&c']) + }) + + it('leaves arguments with backslashes untouched, since zigpty quotes them', () => { + expect(resolvePtyCommand('pnpm', ['src\\**\\*.ts', 'dir\\file(1).txt'], win32).args).toEqual(['/d', '/c', 'pnpm', 'src\\**\\*.ts', 'dir\\file(1).txt']) + expect(resolvePtyCommand('C:\\a(1)\\x.cmd', [], win32).args).toEqual(['/d', '/c', 'C:\\a(1)\\x.cmd']) }) it('skips the cwd when NoDefaultCurrentDirectoryInExePath is set', () => { diff --git a/packages/hub/src/node/pty-command.ts b/packages/hub/src/node/pty-command.ts index aec340fd..fce1e8b9 100644 --- a/packages/hub/src/node/pty-command.ts +++ b/packages/hub/src/node/pty-command.ts @@ -37,6 +37,8 @@ export function resolvePtyCommand( // `.cmd`/`.bat` shims re-parse their arguments // metachar need a second round of escaping to survive both passes const isBatch = ext === '.cmd' || ext === '.bat' + // Known limit: a quoted command path plus a quoted argument makes `cmd /c` + // strip the line's outer quotes. Pass shims by name (`vite`), not by path. return { command: comSpec, args: ['/d', '/c', escapeCmd(command, false), ...args.map(arg => escapeCmd(arg, isBatch))], @@ -54,7 +56,8 @@ function isCmd(command: string, comSpec: string): boolean { } function findExecutable(command: string, cwd: string, env: ResolvePtyCommandOptions['env']): string | undefined { - const extensions = (getEnv(env, 'PATHEXT') || DEFAULT_PATHEXT).split(';').filter(Boolean) + // Windows paths ignore case; lower-case so the lookup also works on case-sensitive filesystems. + const extensions = (getEnv(env, 'PATHEXT') || DEFAULT_PATHEXT).toLowerCase().split(';').filter(Boolean) const candidates = extname(command) ? [command, ...extensions.map(ext => command + ext)] : extensions.map(ext => command + ext) const searchCwd = getEnv(env, 'NoDefaultCurrentDirectoryInExePath') === undefined @@ -73,11 +76,12 @@ function findExecutable(command: string, cwd: string, env: ResolvePtyCommandOpti /** * Escape `cmd.exe` metacharacters in an argument zigpty passes unquoted. - * zigpty already double-quotes arguments with whitespace or quotes, and - * `cmd.exe` keeps quoted metacharacters literal, so those are left alone. + * zigpty double-quotes empty arguments and ones with a space, tab, quote or + * backslash, and `cmd.exe` keeps quoted metacharacters literal, so those + * are left alone - a `^` added there would reach the program as-is. */ function escapeCmd(arg: string, doubleEscape: boolean): string { - if (!arg || /[\s"]/.test(arg)) + if (!arg || /[ \t"\\]/.test(arg)) return arg const escaped = arg.replace(CMD_META_CHARS, '^$&') return doubleEscape ? escaped.replace(CMD_META_CHARS, '^$&') : escaped