diff --git a/src/config.ts b/src/config.ts index a5bc8c2..bc1f0ff 100644 --- a/src/config.ts +++ b/src/config.ts @@ -48,6 +48,14 @@ export interface SidebarConfig { terminalsPerSession?: number /** How long a disconnected terminal process survives awaiting a reconnect. */ reconnectGraceMs?: number + /** + * Terminal shell (absolute path or bare executable name) for BOTH the UI + * terminal tabs and the model-facing `terminal_*` tools. Empty = auto: + * POSIX follows `$SHELL` then the account login shell; Windows follows + * `DSH_SIDEBAR_SHELL`, then probes for `pwsh.exe`, then falls back to the + * inbox `powershell.exe` (5.1). + */ + shell?: string } /** Schemastery schema for the plugin configuration. */ @@ -57,6 +65,7 @@ export const Config: z = z.object({ listLimit: z.number().step(1).min(1).default(1000), terminalsPerSession: z.number().step(1).min(1).default(3), reconnectGraceMs: z.number().step(1).min(0).default(30_000), + shell: z.string().default(''), }) /** Fully defaulted sidebar host settings. */ @@ -66,6 +75,7 @@ export interface ResolvedSidebarConfig { listLimit: number terminalsPerSession: number reconnectGraceMs: number + shell: string } /** @@ -81,6 +91,7 @@ export function resolveSidebarConfig(config: SidebarConfig | undefined): Resolve listLimit: config?.listLimit ?? 1000, terminalsPerSession: config?.terminalsPerSession ?? 3, reconnectGraceMs: config?.reconnectGraceMs ?? 30_000, + shell: config?.shell?.trim() ?? '', } } diff --git a/src/index.ts b/src/index.ts index f9e0622..3933f4f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -426,18 +426,23 @@ export function apply(ctx: Context, config?: SidebarConfig): void { // restore it before any terminal can spawn (idempotent). ensureSpawnHelper() const resolved = resolveSidebarConfig(config) + // One shell resolution feeds BOTH terminal surfaces: the UI tabs and the + // model-facing terminal_* tools. They must stay in lockstep, otherwise a + // configured shell fixes one surface and silently leaves the other on the + // platform default. + const terminalShell = defaultShell({ explicit: resolved.shell }) // The web runtime's bind-derived trust list (boot-sampled LAN literals // plus --trusted-host authorities) — the authoritative source the /api // gateway fence derives its list from. Read per request from the live // service value; a replaced list takes effect without a plugin restart. const fence = (req: SidebarHttpRequest): boolean => isTrustedApiRequest(req, ctx.webRuntime.trustedHosts) - const ptyManager = new PtyManager(defaultShell(), resolved.terminalsPerSession) + const ptyManager = new PtyManager(terminalShell, resolved.terminalsPerSession) // The agent-owned terminal registry: parallel to the UI-tab ptyManager, // keyed by uuid (the model's opaque handle) instead of `${sessionId}:${tabId}`, // uncapped, and torn down with the plugin. The model creates terminals here // through the terminal_create tool; the sidebar view attaches through the // same /sidebar/ws/terminal upgrade with ?uuid=... instead of ?tab=... - const agentPtyRegistry = new AgentPtyRegistry(defaultShell()) + const agentPtyRegistry = new AgentPtyRegistry(terminalShell) // ── User-facing "Side card" preferences ────────────────────────────────── // Register the namespace with the settings provider so the Settings page diff --git a/src/pty-manager.ts b/src/pty-manager.ts index 5b617ec..860145a 100644 --- a/src/pty-manager.ts +++ b/src/pty-manager.ts @@ -193,18 +193,93 @@ export class PtyManager { } /** - * The interactive shell for this platform, resolved like a terminal - * emulator: an explicit `$SHELL` on the dsh process wins (deployment - * override), then the account's login shell from passwd, then `/bin/bash`. - * The passwd step matters because service managers and container inits - * often start dsh without `SHELL`, and the tab should still open the - * user's login shell (e.g. zsh) instead of silently degrading to bash. - * Windows short-circuits to `powershell.exe` before any resolution. + * Inputs for {@link defaultShell} resolution. Every field is optional and + * defaults to the live process, which keeps the no-argument call sites + * working while tests (and exotic embedders) can pin the platform, the + * environment, and the existence probe independently — the Windows chain + * never executes on the ubuntu CI runners, so it is only testable through + * these injection points. */ -export function defaultShell(): string { - if (process.platform === 'win32') return 'powershell.exe' - const envShell = process.env.SHELL - if (envShell !== undefined && envShell.trim() !== '') return envShell +export interface ShellResolutionOptions { + /** Platform override (defaults to `process.platform`). */ + platform?: NodeJS.Platform + /** Environment override; the resolver only reads SHELL, DSH_SIDEBAR_SHELL, PATH, ProgramW6432, ProgramFiles, LOCALAPPDATA. */ + env?: NodeJS.ProcessEnv + /** Explicitly configured shell (the `shell` config field); wins over every automatic source. Empty means unset. */ + explicit?: string + /** File-existence probe override (defaults to `existsSync`). */ + exists?: (path: string) => boolean +} + +/** + * Candidate directories that may contain a `pwsh.exe` on Windows: PATH + * entries first, then the well-known machine/user install locations + * (including preview channels and per-user MSI/portable layouts). The + * machine-scope search reads both `ProgramW6432` and `ProgramFiles` so a + * 32-bit Node process — whose `ProgramFiles` points at `(x86)` — still + * finds a 64-bit PowerShell 7 install. De-duped while preserving priority + * order. + */ +function windowsPwshCandidateDirs(env: NodeJS.ProcessEnv): string[] { + const dirs: string[] = [] + const pathEntries = env.PATH + if (pathEntries !== undefined) { + // The win32 branch always uses the Windows PATH separator; hardcoding it + // keeps the function testable from POSIX runners without a delimiter + // injection point. + for (const entry of pathEntries.split(';')) { + const trimmed = entry.trim() + if (trimmed !== '') dirs.push(trimmed) + } + } + for (const programFiles of [env.ProgramW6432, env.ProgramFiles]) { + if (programFiles === undefined || programFiles.trim() === '') continue + dirs.push(join(programFiles, 'PowerShell', '7')) + dirs.push(join(programFiles, 'PowerShell', '7-preview')) + } + const localAppData = env.LOCALAPPDATA + if (localAppData !== undefined && localAppData.trim() !== '') { + dirs.push(join(localAppData, 'Microsoft', 'PowerShell', '7')) + dirs.push(join(localAppData, 'Microsoft', 'PowerShell', '7-preview')) + dirs.push(join(localAppData, 'Programs', 'PowerShell', '7')) + dirs.push(join(localAppData, 'Programs', 'PowerShell', '7-preview')) + } + return [...new Set(dirs)] +} + +/** + * The interactive shell for this platform, resolved like a terminal + * emulator: an explicitly configured shell (the `shell` config field) wins, + * then `$SHELL` on POSIX (deployment override), then the account's login + * shell from passwd, then `/bin/bash`. The passwd step matters because + * service managers and container inits often start dsh without `SHELL`, and + * the tab should still open the user's login shell (e.g. zsh) instead of + * silently degrading to bash. + * + * Windows previously short-circuited to `powershell.exe` (the inbox 5.1) + * before any resolution, so PowerShell 7 users always got a legacy shell + * without `??`/`?.`/ternary and with poor ANSI/UTF-8 defaults. The Windows + * chain is now: explicit shell → `DSH_SIDEBAR_SHELL` env override → first + * `pwsh.exe` found on PATH or in a known install directory → the 5.1 + * fallback (machines without PowerShell 7 keep working). + */ +export function defaultShell(options: ShellResolutionOptions = {}): string { + const platform = options.platform ?? process.platform + const env = options.env ?? process.env + const exists = options.exists ?? existsSync + const explicit = options.explicit + if (explicit !== undefined && explicit.trim() !== '') return explicit.trim() + if (platform === 'win32') { + const envShell = env.DSH_SIDEBAR_SHELL + if (envShell !== undefined && envShell.trim() !== '') return envShell.trim() + for (const dir of windowsPwshCandidateDirs(env)) { + const candidate = join(dir, 'pwsh.exe') + if (exists(candidate)) return candidate + } + return 'powershell.exe' + } + const envShell = env.SHELL + if (envShell !== undefined && envShell.trim() !== '') return envShell.trim() // userInfo() throws when the uid has no passwd entry (rare chroots); // without a login shell there is nothing better than the bash default. try { diff --git a/tests/plugin-shape.spec.ts b/tests/plugin-shape.spec.ts index 2110edb..1b44d41 100644 --- a/tests/plugin-shape.spec.ts +++ b/tests/plugin-shape.spec.ts @@ -34,6 +34,13 @@ describe('dsh-better-sidebar plugin export shape', () => { expect(resolved.listLimit).toBe(1000) expect(resolved.terminalsPerSession).toBe(3) expect(resolved.reconnectGraceMs).toBe(30_000) + // The terminal shell config defaults to auto-resolution (empty shell = + // the platform chain in defaultShell()). + expect(resolved.shell).toBe('') + const configured = (schema as unknown as { + (input: Record | undefined): Record + })({ shell: 'pwsh.exe' }) + expect(configured.shell).toBe('pwsh.exe') }) it('registers the side card preferences schema with the documented defaults', async () => { diff --git a/tests/unit.spec.ts b/tests/unit.spec.ts index 7c3c0c6..6833715 100644 --- a/tests/unit.spec.ts +++ b/tests/unit.spec.ts @@ -31,6 +31,7 @@ import { wrapOpenPath, type OpenPathInterceptDeps, type OpenPathService } from ' import { registerOpenPathInterception } from '../src/client/intercept.tsx' import type { Context } from '../src/context-types.ts' import { defaultShell, ensureSpawnHelper } from '../src/pty-manager.ts' +import { resolveSidebarConfig } from '../src/config.ts' import { collectBranchIds, countSubagentDescendants, detectNewDirectSubagent, directSubagentCount, rootAncestor, } from '../src/client/subagent-detect.ts' @@ -859,25 +860,59 @@ describe('agent terminal reconciliation', () => { }) describe('pty helpers', () => { - it('prefers an explicit SHELL, then falls back to the account login shell', () => { - if (process.platform === 'win32') { - // defaultShell() short-circuits to powershell.exe before env/passwd - // resolution on Windows; the POSIX fallback chain is asserted below. - expect(defaultShell()).toBe('powershell.exe') - return - } - const previous = process.env.SHELL - try { - process.env.SHELL = '/explicit/zsh' - expect(defaultShell()).toBe('/explicit/zsh') - process.env.SHELL = ' ' - expect(defaultShell()).toBe('/usr/bin/zsh') - delete process.env.SHELL - expect(defaultShell()).toBe('/usr/bin/zsh') - } finally { - if (previous === undefined) delete process.env.SHELL - else process.env.SHELL = previous - } + it('prefers an explicit shell, then SHELL, then the account login shell on POSIX', () => { + // The platform is injected so this chain also runs (and is asserted) on + // non-POSIX developer machines. The userInfo mock at the top of the file + // pins the passwd login shell to /usr/bin/zsh. + expect(defaultShell({ platform: 'linux', env: { SHELL: '/explicit/zsh' } })).toBe('/explicit/zsh') + // Surrounding whitespace must not leak into the spawned executable path. + expect(defaultShell({ platform: 'linux', env: { SHELL: ' /explicit/zsh ' } })).toBe('/explicit/zsh') + expect(defaultShell({ platform: 'linux', env: { SHELL: ' ' } })).toBe('/usr/bin/zsh') + expect(defaultShell({ platform: 'linux', env: {} })).toBe('/usr/bin/zsh') + }) + + it('Windows: explicit shell wins, then DSH_SIDEBAR_SHELL, then a probed pwsh.exe, then powershell.exe 5.1', () => { + // The explicit shell (the `shell` config field) beats every automatic + // source, and whitespace-only values count as unset. + expect(defaultShell({ platform: 'win32', explicit: 'C:\\Tools\\pwsh.exe', env: { DSH_SIDEBAR_SHELL: 'pwsh.exe' } })) + .toBe('C:\\Tools\\pwsh.exe') + expect(defaultShell({ platform: 'win32', explicit: ' ', env: { DSH_SIDEBAR_SHELL: 'pwsh.exe' } })) + .toBe('pwsh.exe') + + // PATH is probed entry by entry; the first directory containing pwsh.exe + // wins and the returned value is the full resolved path. The candidate is + // normalized to forward slashes before comparing so the assertion holds + // under BOTH join() implementations: on the ubuntu runners node:path is + // POSIX and joins 'C:\\other' + 'pwsh.exe' as 'C:\\other/pwsh.exe'. + const fromPath = defaultShell({ + platform: 'win32', + env: { PATH: 'C:\\tools;C:\\other' }, + exists: path => path.replaceAll('\\', '/') === 'C:/other/pwsh.exe', + }) + expect(fromPath.replaceAll('\\', '/')).toBe('C:/other/pwsh.exe') + + // PATH misses fall through to the known install directories. On 32-bit + // Node, ProgramW6432 is the real 64-bit Program Files and is preferred. + const fromProgramW6432 = defaultShell({ + platform: 'win32', + env: { ProgramW6432: 'C:\\PF64' }, + exists: path => path.replaceAll('\\', '/') === 'C:/PF64/PowerShell/7/pwsh.exe', + }) + expect(fromProgramW6432.replaceAll('\\', '/')).toBe('C:/PF64/PowerShell/7/pwsh.exe') + const fromProgramFiles = defaultShell({ + platform: 'win32', + env: { ProgramFiles: 'C:\\PF', LOCALAPPDATA: 'C:\\Users\\x\\AppData\\Local' }, + exists: path => path.replaceAll('\\', '/') === 'C:/PF/PowerShell/7/pwsh.exe', + }) + expect(fromProgramFiles.replaceAll('\\', '/')).toBe('C:/PF/PowerShell/7/pwsh.exe') + + // Nothing installed: keep the inbox 5.1 fallback instead of breaking. + expect(defaultShell({ platform: 'win32', env: {}, exists: () => false })).toBe('powershell.exe') + }) + + it('trims the configured shell and defaults it to auto for old documents', () => { + expect(resolveSidebarConfig(undefined).shell).toBe('') + expect(resolveSidebarConfig({ shell: ' pwsh.exe ' }).shell).toBe('pwsh.exe') }) it('restores the spawn-helper executable bit idempotently', () => {