From 26706bc3e9cf96ef0810a84dad13f856974b0c31 Mon Sep 17 00:00:00 2001 From: HuanLinOTO Date: Tue, 8 Sep 2026 21:21:30 +0800 Subject: [PATCH] =?UTF-8?q?feat(terminal):=20POSIX=20=E4=B9=9F=E9=A2=84?= =?UTF-8?q?=E6=A3=80=20shell=20=E5=8F=AF=E6=89=A7=E8=A1=8C=E6=96=87?= =?UTF-8?q?=E4=BB=B6=EF=BC=8C=E7=BB=9F=E4=B8=80=20shell-not-found=20?= =?UTF-8?q?=E9=94=99=E8=AF=AF=E7=A0=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/pty-manager.ts | 87 ++++++++++++++++++++++++--------------- src/wire.ts | 3 ++ tests/pty-helpers.spec.ts | 38 ++++++++++++++++- 3 files changed, 93 insertions(+), 35 deletions(-) diff --git a/src/pty-manager.ts b/src/pty-manager.ts index 156ecb6..88896de 100644 --- a/src/pty-manager.ts +++ b/src/pty-manager.ts @@ -352,51 +352,72 @@ export function resolveShellExecutable( shell: string, options: ShellExecutableResolutionOptions = {}, ): string { - const configured = shell.trim() + const configured = unquotePath(shell.trim()) const platform = options.platform ?? process.platform - if (platform !== 'win32' || configured === '') return configured + if (configured === '') return configured const env = options.env ?? process.env const exists = options.exists ?? existsSync - const rawPathext = windowsEnv(env, 'PATHEXT') - const executableExts = (rawPathext ?? '.COM;.EXE') - .split(';') - .map(extension => extension.trim()) - // node-pty ultimately calls CreateProcess; batch files need an - // intermediate cmd.exe and therefore are not valid shell executables. - .filter(extension => /^\.(?:com|exe)$/i.test(extension)) - if (executableExts.length === 0) executableExts.push('.EXE', '.COM') + const notFound = (): SidebarError => + new SidebarError('shell-not-found', `shell executable not found: "${configured}"`, 400, { shell: configured }) - const hasExtension = win32Path.extname(configured) !== '' - const names = hasExtension - ? [configured] - : executableExts.map(extension => configured + extension.toLowerCase()) - const hasPath = win32Path.isAbsolute(configured) || /[\\/]/.test(configured) - const candidates: string[] = [] - if (hasPath) { - candidates.push(...names) - } else { - const path = windowsEnv(env, 'PATH') - if (path !== undefined) { - for (const dir of path.split(';').map(entry => entry.trim()).filter(Boolean)) { - for (const name of names) candidates.push(win32Path.join(dir, name)) + if (platform === 'win32') { + const rawPathext = windowsEnv(env, 'PATHEXT') + const executableExts = (rawPathext ?? '.COM;.EXE') + .split(';') + .map(extension => extension.trim()) + // node-pty ultimately calls CreateProcess; batch files need an + // intermediate cmd.exe and therefore are not valid shell executables. + .filter(extension => /^\.(?:com|exe)$/i.test(extension)) + if (executableExts.length === 0) executableExts.push('.EXE', '.COM') + + const hasExtension = win32Path.extname(configured) !== '' + const names = hasExtension + ? [configured] + : executableExts.map(extension => configured + extension.toLowerCase()) + const hasPath = win32Path.isAbsolute(configured) || /[\\/]/.test(configured) + const candidates: string[] = [] + if (hasPath) { + candidates.push(...names) + } else { + const path = windowsEnv(env, 'PATH') + if (path !== undefined) { + for (const dir of path.split(';').map(entry => entry.trim()).filter(Boolean)) { + for (const name of names) candidates.push(win32Path.join(dir, name)) + } + } + const systemRoot = windowsEnv(env, 'SystemRoot') + if (systemRoot !== undefined && systemRoot.trim() !== '') { + for (const name of names) candidates.push(win32Path.join(systemRoot, 'System32', name)) + } + if (/^pwsh(?:\.exe)?$/i.test(configured)) { + for (const dir of windowsPwshCandidateDirs(env)) { + candidates.push(win32Path.join(dir, 'pwsh.exe')) + } } } - const systemRoot = windowsEnv(env, 'SystemRoot') - if (systemRoot !== undefined && systemRoot.trim() !== '') { - for (const name of names) candidates.push(win32Path.join(systemRoot, 'System32', name)) - } - if (/^pwsh(?:\.exe)?$/i.test(configured)) { - for (const dir of windowsPwshCandidateDirs(env)) { - candidates.push(win32Path.join(dir, 'pwsh.exe')) - } + + for (const candidate of [...new Set(candidates)]) { + if (exists(candidate)) return candidate } + throw notFound() } - for (const candidate of [...new Set(candidates)]) { + // POSIX: the previous pass-through delegated a wrong name to execvp and the + // pty died with a bare "[process exited with code N]". Probe like Windows: + // a path with a separator must exist; a bare name is searched along PATH + // (the colon form is fixed by the platform). A miss is a clear, actionable + // error instead of a cryptic exit code. + if (configured.includes('/')) { + if (!exists(configured)) throw notFound() + return configured + } + const path = env.PATH ?? '/usr/bin:/bin' + for (const dir of path.split(':').map(entry => entry.trim()).filter(Boolean)) { + const candidate = join(dir, configured) if (exists(candidate)) return candidate } - throw new SidebarError('pty-error', `shell executable not found: "${configured}"`) + throw notFound() } /** diff --git a/src/wire.ts b/src/wire.ts index ccea91c..3002543 100644 --- a/src/wire.ts +++ b/src/wire.ts @@ -17,6 +17,7 @@ export type SidebarErrorCode = | 'git-error' | 'pty-error' | 'pty-deps-missing' + | 'shell-not-found' | 'job-error' | 'sidechat-error' | 'subagents-unavailable' @@ -30,6 +31,8 @@ export class SidebarError extends Error { readonly code: SidebarErrorCode, message: string, readonly status = 400, + /** Optional structured context (e.g. `{ shell }` for shell-not-found). */ + readonly meta?: Record, ) { super(message) } diff --git a/tests/pty-helpers.spec.ts b/tests/pty-helpers.spec.ts index 02f80f0..87cf049 100644 --- a/tests/pty-helpers.spec.ts +++ b/tests/pty-helpers.spec.ts @@ -25,6 +25,7 @@ import { splitShellArgs, unquotePath, } from '../src/pty-manager.ts' +import { SidebarError } from '../src/wire.ts' describe('pty helpers', () => { it('prefers an explicit shell, then SHELL, then the account login shell on POSIX', () => { @@ -114,8 +115,41 @@ describe('pty helpers', () => { .toThrow('shell executable not found: "missing-shell"') }) - it('keeps POSIX bare shell resolution delegated to execvp', () => { - expect(resolveShellExecutable(' zsh ', { platform: 'linux', env: {}, exists: () => false })).toBe('zsh') + it('POSIX: resolves bare names along PATH and names a missing configured shell', () => { + const options = { + platform: 'linux' as const, + env: { PATH: '/usr/local/bin:/usr/bin' }, + exists: (path: string) => path === join('/usr/bin', 'zsh'), + } + expect(resolveShellExecutable('zsh', options)).toBe(join('/usr/bin', 'zsh')) + const thrown = (() => { + try { + resolveShellExecutable('nope', options) + return undefined + } catch (error) { + return error + } + })() + expect(thrown).toBeInstanceOf(SidebarError) + expect((thrown as SidebarError).code).toBe('shell-not-found') + expect((thrown as SidebarError).message).toBe('shell executable not found: "nope"') + expect((thrown as SidebarError).meta?.shell).toBe('nope') + }) + + it('POSIX: checks an absolute path exists and passes it through verbatim', () => { + expect(resolveShellExecutable('/explicit/zsh', { platform: 'linux', env: {}, exists: () => true })) + .toBe('/explicit/zsh') + expect(() => resolveShellExecutable('/missing/zsh', { platform: 'linux', env: {}, exists: () => false })) + .toThrow('shell executable not found: "/missing/zsh"') + }) + + it('unquotes the configured shell before resolving (Windows probe included)', () => { + const options = { + platform: 'win32' as const, + env: { PATH: 'C:\\Tools' }, + exists: (path: string) => path.replaceAll('\\', '/') === 'C:/Tools/pwsh.exe', + } + expect(resolveShellExecutable('"pwsh.exe"', options).replaceAll('\\', '/')).toBe('C:/Tools/pwsh.exe') }) it('trims the configured shell and defaults it to auto for old documents', () => {