diff --git a/apps/desktop/electron/main.cjs b/apps/desktop/electron/main.cjs index ce8e4bb83ca8..639c085cbec8 100644 --- a/apps/desktop/electron/main.cjs +++ b/apps/desktop/electron/main.cjs @@ -51,6 +51,7 @@ const { resolveReadableFileForIpc, resolveTimeoutMs } = require('./hardening.cjs') +const { resolveShellCommand } = require('./terminal-shell.cjs') let nodePty = null @@ -75,6 +76,29 @@ try { } } +// node-pty's macOS prebuild ships spawn-helper without the execute bit. +// posix_spawnp on macOS spawns the helper first (it then launches the real +// shell), so the helper must be executable. Fix it at load time so the +// terminal works on every `npm run dev` / packaged run without manual chmod. +;(function ensurePtySpawnHelperExecutable() { + if (!nodePty || process.platform !== 'darwin') return + try { + const ptyPkgRoot = path.dirname(require.resolve('node-pty/package.json')) + const helperPath = path.join(ptyPkgRoot, 'prebuilds', `${process.platform}-${process.arch}`, 'spawn-helper') + if (!fs.existsSync(helperPath)) return + const stat = fs.statSync(helperPath) + // eslint-disable-next-line no-bitwise + if (!(stat.mode & fs.constants.S_IXUSR)) { + // eslint-disable-next-line no-bitwise + fs.chmodSync(helperPath, stat.mode | fs.constants.S_IXUSR | fs.constants.S_IXGRP | fs.constants.S_IXOTH) + rememberLog(`[pty] made spawn-helper executable: ${helperPath}`) + } + } catch (error) { + // Non-fatal — terminal:start will surface a diagnostic error downstream. + rememberLog(`[pty] could not ensure spawn-helper is executable: ${error.message}`) + } +})() + const USER_DATA_OVERRIDE = process.env.HERMES_DESKTOP_USER_DATA_DIR if (USER_DATA_OVERRIDE) { const resolvedUserData = path.resolve(USER_DATA_OVERRIDE) @@ -5141,19 +5165,22 @@ function findGitRoot(start) { } function terminalShellCommand() { - if (IS_WINDOWS) { - return { args: [], command: process.env.COMSPEC || 'cmd.exe' } - } + const result = resolveShellCommand({ + platform: process.platform, + envSHELL: process.env.SHELL, + envComspec: process.env.COMSPEC + }) - const configuredShell = process.env.SHELL || '' - const shellPath = - (path.isAbsolute(configuredShell) && fs.existsSync(configuredShell) && configuredShell) || - ['/bin/zsh', '/bin/bash', '/bin/sh'].find(candidate => fs.existsSync(candidate)) || - '/bin/sh' - const shellName = path.basename(shellPath) - const interactiveArgs = shellName.includes('zsh') || shellName.includes('bash') ? ['-il'] : ['-i'] + // Defensive: if the resolved shell doesn't exist on disk, log a clear + // diagnostic so the downstream posix_spawnp error is debuggable. + if (!fs.existsSync(result.command)) { + rememberLog( + `[terminal] resolved shell does not exist on disk: ${result.command} ` + + `(SHELL=${process.env.SHELL || ''})` + ) + } - return { args: interactiveArgs, command: shellPath, name: shellName } + return result } function safeTerminalCwd(cwd) { @@ -5269,13 +5296,32 @@ ipcMain.handle('hermes:terminal:start', async (event, payload = {}) => { const cwd = safeTerminalCwd(payload?.cwd) const cols = Math.max(2, Number.parseInt(String(payload?.cols || 80), 10) || 80) const rows = Math.max(2, Number.parseInt(String(payload?.rows || 24), 10) || 24) - const ptyProcess = nodePty.spawn(command, args, { - cols, - cwd, - env: terminalShellEnv(), - name: 'xterm-256color', - rows - }) + + let ptyProcess + try { + ptyProcess = nodePty.spawn(command, args, { + cols, + cwd, + env: terminalShellEnv(), + name: 'xterm-256color', + rows + }) + } catch (error) { + const existsOnDisk = fs.existsSync(command) + const shellStat = existsOnDisk ? fs.statSync(command) : null + const details = [ + `shell=${command}`, + `args=[${args.join(', ')}]`, + `cwd=${cwd}`, + `exists=${existsOnDisk}`, + `mode=${shellStat ? (shellStat.mode & 0o777).toString(8) : 'n/a'}`, + `SHELL=${process.env.SHELL || ''}`, + `platform=${process.platform}`, + `arch=${process.arch}` + ].join(' ') + rememberLog(`[terminal] spawn failed: ${error.message} | ${details}`) + throw new Error(`Failed to start terminal: ${error.message} (${details})`) + } terminalSessions.set(id, { pty: ptyProcess, webContentsId: event.sender.id }) diff --git a/apps/desktop/electron/terminal-shell.cjs b/apps/desktop/electron/terminal-shell.cjs new file mode 100644 index 000000000000..820869e78047 --- /dev/null +++ b/apps/desktop/electron/terminal-shell.cjs @@ -0,0 +1,52 @@ +/** + * terminal-shell.cjs + * + * Resolve the interactive shell command, args, and display name for the + * terminal PTY. Pure — no `require('electron')` — so it can be unit-tested + * with `node --test` (same pattern as connection-config.cjs / bootstrap-platform.cjs). + * + * macOS / Linux resolution order: + * 1. process.env.SHELL (if absolute + exists) + * 2. /bin/zsh → /bin/bash → /bin/sh (first one that exists) + * 3. /bin/sh (last-resort fallback) + * + * Windows: process.env.COMSPEC or cmd.exe. + */ + +const fs = require('node:fs') +const path = require('node:path') + +/** + * @param {object} opts + * @param {string} opts.platform — process.platform ('darwin' | 'linux' | 'win32') + * @param {string} [opts.envSHELL] — process.env.SHELL (unix) + * @param {string} [opts.envComspec] — process.env.COMSPEC (windows) + * @param {((p: string) => boolean)} [opts.existsSync] — fs.existsSync, injectable for tests + * @returns {{ command: string, args: string[], name: string }} + */ +function resolveShellCommand(opts = {}) { + const { platform } = opts + const existsSync = opts.existsSync || fs.existsSync + + if (platform === 'win32') { + const comspec = opts.envComspec || '' + const command = + (path.isAbsolute(comspec) && existsSync(comspec) && comspec) || 'cmd.exe' + return { args: [], command, name: path.basename(command) } + } + + // macOS / Linux: prefer SHELL, then common fallbacks. + const configuredShell = opts.envSHELL || '' + const shellPath = + (path.isAbsolute(configuredShell) && existsSync(configuredShell) && configuredShell) || + ['/bin/zsh', '/bin/bash', '/bin/sh'].find(candidate => existsSync(candidate)) || + '/bin/sh' + + const shellName = path.basename(shellPath) + const interactiveArgs = + shellName.includes('zsh') || shellName.includes('bash') ? ['-il'] : ['-i'] + + return { args: interactiveArgs, command: shellPath, name: shellName } +} + +module.exports = { resolveShellCommand } diff --git a/apps/desktop/electron/terminal-shell.test.cjs b/apps/desktop/electron/terminal-shell.test.cjs new file mode 100644 index 000000000000..f89160e7e6ff --- /dev/null +++ b/apps/desktop/electron/terminal-shell.test.cjs @@ -0,0 +1,154 @@ +const assert = require('node:assert/strict') +const test = require('node:test') + +const { resolveShellCommand } = require('./terminal-shell.cjs') + +// Stub existsSync — returns true only for paths in this set. +function stubExistsSync(present) { + return p => present.has(p) +} + +test('macOS: uses SHELL when absolute and exists', () => { + const result = resolveShellCommand({ + platform: 'darwin', + envSHELL: '/bin/zsh', + existsSync: stubExistsSync(new Set(['/bin/zsh'])) + }) + assert.equal(result.command, '/bin/zsh') + assert.deepEqual(result.args, ['-il']) + assert.equal(result.name, 'zsh') +}) + +test('macOS: falls back to /bin/zsh when SHELL is unset', () => { + const result = resolveShellCommand({ + platform: 'darwin', + envSHELL: '', + existsSync: stubExistsSync(new Set(['/bin/zsh'])) + }) + assert.equal(result.command, '/bin/zsh') +}) + +test('macOS: falls back to /bin/bash when SHELL is invalid and zsh missing', () => { + const result = resolveShellCommand({ + platform: 'darwin', + envSHELL: '/nonexistent/shell', + existsSync: stubExistsSync(new Set(['/bin/bash'])) + }) + assert.equal(result.command, '/bin/bash') +}) + +test('macOS: falls back to /bin/sh when SHELL is relative', () => { + const result = resolveShellCommand({ + platform: 'darwin', + envSHELL: 'zsh', + existsSync: stubExistsSync(new Set(['/bin/sh'])) + }) + assert.equal(result.command, '/bin/sh') +}) + +test('macOS: returns /bin/sh as last resort even if it does not exist', () => { + const result = resolveShellCommand({ + platform: 'darwin', + envSHELL: '', + existsSync: stubExistsSync(new Set()) + }) + assert.equal(result.command, '/bin/sh') +}) + +test('macOS: returns interactive login args for zsh', () => { + const result = resolveShellCommand({ + platform: 'darwin', + envSHELL: '/bin/zsh', + existsSync: stubExistsSync(new Set(['/bin/zsh'])) + }) + assert.deepEqual(result.args, ['-il']) +}) + +test('macOS: returns interactive login args for bash', () => { + const result = resolveShellCommand({ + platform: 'darwin', + envSHELL: '/bin/bash', + existsSync: stubExistsSync(new Set(['/bin/bash'])) + }) + assert.deepEqual(result.args, ['-il']) +}) + +test('macOS: returns -i (not -il) for non-zsh/bash shells', () => { + const result = resolveShellCommand({ + platform: 'darwin', + envSHELL: '/bin/fish', + existsSync: stubExistsSync(new Set(['/bin/fish'])) + }) + assert.deepEqual(result.args, ['-i']) +}) + +test('linux: same fallback chain as macOS', () => { + const result = resolveShellCommand({ + platform: 'linux', + envSHELL: '', + existsSync: stubExistsSync(new Set(['/bin/bash'])) + }) + assert.equal(result.command, '/bin/bash') + assert.deepEqual(result.args, ['-il']) +}) + +test('windows: uses COMSPEC when absolute and exists', () => { + const result = resolveShellCommand({ + platform: 'win32', + // A path that path.isAbsolute() returns true for on the host OS running + // the test (typically Unix). On real Windows this would be C:\… instead. + envComspec: '/opt/cmd.exe', + existsSync: stubExistsSync(new Set(['/opt/cmd.exe'])) + }) + assert.equal(result.command, '/opt/cmd.exe') + assert.deepEqual(result.args, []) + assert.equal(result.name, 'cmd.exe') +}) + +test('windows: falls back to cmd.exe when COMSPEC is unset', () => { + const result = resolveShellCommand({ + platform: 'win32', + envComspec: '', + existsSync: stubExistsSync(new Set()) + }) + assert.equal(result.command, 'cmd.exe') + assert.deepEqual(result.args, []) + assert.equal(result.name, 'cmd.exe') +}) + +test('windows: falls back to cmd.exe when COMSPEC is relative', () => { + const result = resolveShellCommand({ + platform: 'win32', + envComspec: 'cmd.exe', + existsSync: stubExistsSync(new Set()) + }) + assert.equal(result.command, 'cmd.exe') + assert.deepEqual(result.args, []) +}) + +test('returns absolute paths only', () => { + // When SHELL is set to an existing absolute path, it stays absolute. + const zsh = resolveShellCommand({ + platform: 'darwin', + envSHELL: '/bin/zsh', + existsSync: stubExistsSync(new Set(['/bin/zsh'])) + }) + assert.ok(zsh.command.startsWith('/')) + + // Fallback also returns absolute paths. + const fallback = resolveShellCommand({ + platform: 'darwin', + envSHELL: '', + existsSync: stubExistsSync(new Set(['/bin/zsh'])) + }) + assert.ok(fallback.command.startsWith('/')) + + // Even the last resort is absolute. + const last = resolveShellCommand({ + platform: 'darwin', + envSHELL: '', + existsSync: stubExistsSync(new Set()) + }) + assert.equal(last.command, '/bin/sh') + assert.ok(last.command.startsWith('/')) +})