feat: configurable terminal shell with Windows pwsh auto-probe - #95
Open
oy-g wants to merge 2 commits into
Open
Conversation
Windows previously hardcoded powershell.exe (5.1) in defaultShell(), ignoring PowerShell 7 installs. Resolve the shell in priority order: explicit shell config -> DSH_SIDEBAR_SHELL -> PATH/known-dir probe for pwsh.exe -> inbox 5.1 fallback. The same resolved shell feeds both the UI terminal tabs (PtyManager) and the model-facing terminal_* tools (AgentPtyRegistry). defaultShell() now accepts injectable platform/env/exists options so the Windows chain is unit-testable on the ubuntu CI, which never executes win32 branches. The probe also checks ProgramW6432 for 32-bit Node, and machines without PowerShell 7 keep the 5.1 fallback. Refs: omdsh-dev#46
There was a problem hiding this comment.
Pull request overview
This PR adds a configurable terminal shell selection mechanism so Windows users can automatically prefer PowerShell 7 (pwsh.exe) (or explicitly configure a shell), while ensuring the UI terminal tabs and model-facing terminal_* tools use the same resolved shell.
Changes:
- Refactors
defaultShell()to accept injectable resolution inputs and adds a Windows probe chain (shellconfig →DSH_SIDEBAR_SHELL→ probepwsh.exe→ fallbackpowershell.exe). - Introduces a new
shellfield in plugin config (default''= auto-resolution). - Updates wiring and tests so both terminal surfaces share one resolution and the Windows chain is unit-tested via injection.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/pty-manager.ts |
Adds injectable shell resolution options + Windows pwsh.exe probing and updated resolution order. |
src/config.ts |
Adds shell config field, schema default, and trimming in resolved config. |
src/index.ts |
Computes shell once and passes it to both PtyManager and AgentPtyRegistry to keep behavior consistent. |
tests/unit.spec.ts |
Adds injected-platform tests covering POSIX chain and full Windows chain behavior. |
tests/plugin-shape.spec.ts |
Asserts config schema default and override behavior for the new shell field. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
281
to
282
| const envShell = 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, ProgramFiles, LOCALAPPDATA. */ |
- POSIX SHELL is validated with trim() but was returned untrimmed; trailing whitespace could leak into the spawned executable path. Return the trimmed value like the explicit/config and Windows/env branches. - ShellResolutionOptions.env comment now lists ProgramW6432, which the Windows probe prefers over ProgramFiles on 32-bit Node. - Test pins the SHELL trimming behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat: configurable terminal shell — Windows pwsh 探测 + shell 配置
问题
defaultShell()在 Windows 上硬编码返回powershell.exe（5.1）：装了 PowerShell 7 的用户仍然拿到 5.1，且没有任何配置出口。该函数同时决定两条终端路径：侧边栏 UI 终端 tab 和注入给模型的terminal_*工具。方案
解析优先级：
shell配置字段（新增）——显式配置最高；DSH_SIDEBAR_SHELL；pwsh.exe（PATH →ProgramFiles/ProgramW6432/LOCALAPPDATA下的已知安装目录，含7-preview），命中返回完整路径；powershell.exe（5.1）——没装 pwsh 的机器行为不变；$SHELL→ login shell →/bin/bash）。defaultShell()增加可注入参数（platform/env/exists/explicit），使 Windows 解析链能在 ubuntu CI 上被单测真实执行（CI 不跑 win32 分支）。改动
src/pty-manager.tsdefaultShell重构 + Windows 探测链src/config.tsshell配置字段（默认空 = 自动）src/index.tstests/unit.spec.tstests/plugin-shape.spec.ts用法
不配置时：装了 pwsh 7 的机器自动用 pwsh；没装的保持 5.1。
验证
pnpm typecheck✅；聚焦测试 114/115（唯一失败为 Windows 本机既有失败，已确认与本次无关）；tsc + tsdown构建 ✅。link:安装实测：UI 终端与模型terminal_*工具均打开 PowerShell 7.6.3。check:consumer-types与真实 DSH 挂载冒烟由 CI 的 ubuntu job 覆盖（依赖 bash 脚本，Windows 本机无法运行）。讨论
shell目前是 host 配置（cordis.patch.yml，重启生效）；是否升级为用户级设置项（设置页可见）可后续讨论。