fix(terminal): use detected shell for execa and vscode terminal creation (#321)#333
fix(terminal): use detected shell for execa and vscode terminal creation (#321)#333F915 wants to merge 11 commits into
Conversation
…ion (Zoo-Code-Org#321) Connect the existing getShell() to both terminal creation paths to fix the Windows shell mismatch where AI generates pwsh syntax but commands execute in cmd.exe. - ExecaTerminalProcess: use getShell() instead of shell: true - Terminal: pass shellPath: getShell() to createTerminal Fixes Zoo-Code-Org#321
…gration (Zoo-Code-Org#321) Update two test cases that previously expected shell: true to now expect shell: expect.any(String), reflecting the change from shell: true to getShell() in ExecaTerminalProcess. Refs Zoo-Code-Org#321 Co-authored-by: Zoo Code Contributor <contributor@zoocode.dev>
📝 WalkthroughWalkthroughShell resolution now prefers ChangesTerminal shell & integration changes
Sequence Diagram(s)sequenceDiagram
participant VSCode as VSCode API
participant Terminal as Terminal (constructor)
participant ShellReady as shellIntegrationReady
participant ExecaProc as ExecaTerminalProcess
participant WSL as wsl.exe
VSCode->>Terminal: createTerminal (may omit shellPath for WSL)
Terminal->>ShellReady: wait for onDidChangeTerminalShellIntegration / poll
ShellReady->>ExecaProc: run(command) after readiness
ExecaProc->>ExecaProc: resolve effectiveShell = getExecaShellPath() || getShell()
alt effectiveShell == WSL_EXE_PATH
ExecaProc->>WSL: spawn wsl.exe with ["bash","-lc", command] (+ --cd converted CWD)
else
ExecaProc->>ExecaProc: execa(..., { shell: effectiveShell })
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
F915
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f5fb7a1-993e-4d1d-987f-4466eb7afe09
📒 Files selected for processing (3)
src/integrations/terminal/ExecaTerminalProcess.tssrc/integrations/terminal/Terminal.tssrc/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts
Mock getShell() to verify the fallback actually uses the detected shell instead of just checking for any string value. Suggested-by: CodeRabbit
…ll() integration (Zoo-Code-Org#321) PR Zoo-Code-Org#333 added shellPath and iconPath to Terminal constructor but TerminalRegistry.spec.ts assertions were not updated, causing 4 test failures on ubuntu-latest CI.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/integrations/terminal/__tests__/TerminalRegistry.spec.ts (1)
39-40: ⚡ Quick winAssert
getShell()invocation to prevent hardcoded-shell regressions.You already assert the resulting
shellPath, but addingexpect(shellUtils.getShell).toHaveBeenCalled()makes this test fail if production code hardcodes the same string instead of calling the resolver.Suggested test hardening
it("creates terminal with PAGER set appropriately for platform", () => { TerminalRegistry.createTerminal("/test/path", "vscode") + expect(shellUtils.getShell).toHaveBeenCalledTimes(1) expect(mockCreateTerminal).toHaveBeenCalledWith({ cwd: "/test/path", name: "Roo Code", iconPath: expect.objectContaining({ id: expect.any(String) }), env: { PAGER, ROO_ACTIVE: "true", VTE_VERSION: "0", PROMPT_EOL_MARK: "", }, shellPath: "/mock/fallback-shell", }) })Also applies to: 60-61, 83-84, 107-108, 130-131
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/integrations/terminal/__tests__/TerminalRegistry.spec.ts` around lines 39 - 40, Add assertions that the shell resolver is actually invoked: after each vi.spyOn(shellUtils, "getShell").mockReturnValue("/mock/fallback-shell") in the TerminalRegistry.spec tests, add expect(shellUtils.getShell).toHaveBeenCalled() (or toHaveBeenCalledTimes(1)) to ensure production code calls shellUtils.getShell rather than hardcoding the path; apply this to the occurrences around the tests that set up mocked shells (the blocks using vi.spyOn(shellUtils, "getShell") and asserting shellPath).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/integrations/terminal/__tests__/TerminalRegistry.spec.ts`:
- Around line 39-40: Add assertions that the shell resolver is actually invoked:
after each vi.spyOn(shellUtils,
"getShell").mockReturnValue("/mock/fallback-shell") in the TerminalRegistry.spec
tests, add expect(shellUtils.getShell).toHaveBeenCalled() (or
toHaveBeenCalledTimes(1)) to ensure production code calls shellUtils.getShell
rather than hardcoding the path; apply this to the occurrences around the tests
that set up mocked shells (the blocks using vi.spyOn(shellUtils, "getShell") and
asserting shellPath).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 11da761d-d24d-4448-bec2-2eb4363d25fa
📒 Files selected for processing (1)
src/integrations/terminal/__tests__/TerminalRegistry.spec.ts
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
proyectoauraorg
left a comment
proyectoauraorg
left a comment
There was a problem hiding this comment.
LGTM! Using the detected shell for both execa and VS Code terminal creation resolves the shell mismatch issue on Windows. Clean fix.
| const env = Terminal.getEnv() | ||
| const iconPath = new vscode.ThemeIcon("rocket") | ||
| this.terminal = terminal ?? vscode.window.createTerminal({ cwd, name: "Roo Code", iconPath, env }) | ||
| this.terminal = terminal ?? vscode.window.createTerminal({ cwd, name: "Roo Code", iconPath, env, shellPath: getShell() }) |
There was a problem hiding this comment.
If the user has WSL configured but no explicit shell path, getShell() returns /bin/bash (a Unix path) — does passing that as shellPath actually work on Windows, or would wsl.exe be needed here?
There was a problem hiding this comment.
Good catch, and you're right that this was a real issue.
After further analysis, we've addressed this in the follow-up refactoring (commit 387ca89). The terminal constructor now checks for WSL configuration first — when WSL is detected, shellPath is omitted so VS Code's own WSL profile handles shell selection. Passing an explicit shell path would bypass VS Code's profile system.
With this fix, the inline terminal path can now correctly execute WSL commands. However, the external terminal path still cannot work with WSL, because VS Code shell integration does not support WSL — the supported shells on Windows are limited to Git Bash and pwsh. In practice, the external terminal will wait for shell integration until timeout, then fall back to the inline terminal — which is the expected behavior.
These changes are part of a broader consolidation of shell detection that the WSL issue prompted us to revisit. The upstream code manually parsed VS Code's terminal profile configuration with a shell allowlist, but this approach was inherently fragile — it could not reliably match VS Code's internal shell resolution, which is exactly the kind of mismatch your comment pointed out. To address this at the root, we removed the allowlist and manual parsing in favor of vscode.env.shell, the API VS Code has provided for this purpose. As a result, the two command execution paths now share the same shell resolution logic, so there is no longer a risk of falling back to a different shell than the one the external terminal used. A safety check that guarded against this mismatch — but relied on the same fragile detection it was meant to protect against — became redundant and was removed.
We also removed a sendText call that previously fired before the fallback when shell integration was unavailable — this could cause the command to execute in the external terminal without output capture, and then again via the inline path, resulting in unintended double execution. The fallback to the inline terminal now happens directly.
Thanks again for taking the time to review — the feedback was helpful in tracing this issue to its root cause.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts (1)
176-212: 💤 Low valueUnusual semicolon-prefixed statement formatting.
Lines 180, 189, 198, and 207 use a semicolon prefix before calling
trimRetrievedOutput(). This is syntactically valid but unconventional and reduces readability.♻️ Cleaner formatting
it("clears buffer when all output has been retrieved", () => { terminalProcess["fullOutput"] = "test output data" terminalProcess["lastRetrievedIndex"] = 16 - ; (terminalProcess as any).trimRetrievedOutput() + ;(terminalProcess as any).trimRetrievedOutput() expect(terminalProcess["fullOutput"]).toBe("")Or wrap the call to avoid ASI ambiguity:
- terminalProcess["lastRetrievedIndex"] = 16 - ; (terminalProcess as any).trimRetrievedOutput() + terminalProcess["lastRetrievedIndex"] = 16 + const instance = terminalProcess as any + instance.trimRetrievedOutput()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts` around lines 176 - 212, The tests use unconventional semicolon-prefixed calls like ;(terminalProcess as any).trimRetrievedOutput() which is unnecessary and harms readability; update each test to call the method directly (e.g., (terminalProcess as any).trimRetrievedOutput()) or otherwise wrap the expression to avoid the leading semicolon—modify the four occurrences that invoke trimRetrievedOutput on the terminalProcess instance to remove the prefixed semicolons.src/utils/__tests__/shell.spec.ts (1)
203-263: 💤 Low valueConsider restoring the original
getConfigurationmock after eachgetWslProfiletest.The tests directly reassign
vscode.workspace.getConfigurationwithout restoring it, which could cause test pollution if tests run in a different order or if new tests are added to this describe block.♻️ Suggested improvement
describe("getWslProfile()", () => { + let originalGetConfiguration: typeof vscode.workspace.getConfiguration + beforeEach(() => { Object.defineProperty(process, "platform", { value: "win32" }) + originalGetConfiguration = vscode.workspace.getConfiguration + }) + + afterEach(() => { + vscode.workspace.getConfiguration = originalGetConfiguration })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/__tests__/shell.spec.ts` around lines 203 - 263, The tests for getWslProfile directly overwrite vscode.workspace.getConfiguration and never restore it, risking test pollution; update the describe("getWslProfile") block to save the original getConfiguration (e.g., const originalGetConfig = vscode.workspace.getConfiguration) in a beforeEach and restore it in an afterEach (or use jest.spyOn(vscode.workspace, "getConfiguration") and call mockRestore() in afterEach) so each it(...) can safely mock getConfiguration without leaking state across tests.src/integrations/terminal/ExecaTerminalProcess.ts (1)
37-88: 💤 Low value
effectiveCommandis assigned but unused in the WSL branch.The variable
effectiveCommandon line 47 is only used in the non-WSL else branch (line 87), but it's assigned regardless. This is minor dead code whenisWslShellis true.♻️ Suggested cleanup
const resolvedShell = BaseTerminal.getExecaShellPath() || getShell() const isWslShell = resolvedShell === WSL_EXE_PATH - let effectiveShell: string | boolean = resolvedShell - let effectiveCommand = command - if (isWslShell) { // Spawn wsl.exe directly (not through cmd.exe) to avoid nested-quoting issues. // execa(file, args, options) passes args as an array — no shell interpretation. @@ ... @@ } else { this.subprocess = execa({ - shell: effectiveShell, + shell: resolvedShell, cwd: this.terminal.getCurrentWorkingDirectory(), all: true, stdin: "ignore", env: { ...process.env, LANG: "en_US.UTF-8", LC_ALL: "en_US.UTF-8", }, - })`${effectiveCommand}` + })`${command}` }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/integrations/terminal/ExecaTerminalProcess.ts` around lines 37 - 88, The run method assigns effectiveCommand but never uses it in the WSL branch; remove the dead assignment by either moving the line that sets effectiveCommand into the non-WSL else branch or stop using effectiveCommand entirely and pass command directly to the non-WSL execa template literal—update the run function (referencing run, effectiveCommand, isWslShell, and the execa calls) so effectiveCommand is only created/used where needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/integrations/terminal/__tests__/TerminalProcessExec.cmd.spec.ts`:
- Around line 52-54: The env object literal sets shell to
"C:\Windows\System32\cmd.exe" with unescaped backslashes; update the shell
string in the test (the env object, shell property) to use escaped backslashes
("C:\\Windows\\System32\\cmd.exe") so the path is a valid JavaScript string
literal and avoids accidental escape sequences.
In `@src/integrations/terminal/__tests__/TerminalProcessExec.pwsh.spec.ts`:
- Around line 52-54: The Windows path string assigned to env.shell currently
uses single backslashes which are interpreted as escape sequences; update the
test to use escaped backslashes (e.g., double backslashes) in the "shell" value
so the path reads correctly (replace "C:\Program Files\PowerShell�\pwsh.exe"
with an escaped form); locate the env object in the
TerminalProcessExec.pwsh.spec.ts test and change the shell string to use "\\"
for each backslash.
---
Nitpick comments:
In `@src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts`:
- Around line 176-212: The tests use unconventional semicolon-prefixed calls
like ;(terminalProcess as any).trimRetrievedOutput() which is unnecessary and
harms readability; update each test to call the method directly (e.g.,
(terminalProcess as any).trimRetrievedOutput()) or otherwise wrap the expression
to avoid the leading semicolon—modify the four occurrences that invoke
trimRetrievedOutput on the terminalProcess instance to remove the prefixed
semicolons.
In `@src/integrations/terminal/ExecaTerminalProcess.ts`:
- Around line 37-88: The run method assigns effectiveCommand but never uses it
in the WSL branch; remove the dead assignment by either moving the line that
sets effectiveCommand into the non-WSL else branch or stop using
effectiveCommand entirely and pass command directly to the non-WSL execa
template literal—update the run function (referencing run, effectiveCommand,
isWslShell, and the execa calls) so effectiveCommand is only created/used where
needed.
In `@src/utils/__tests__/shell.spec.ts`:
- Around line 203-263: The tests for getWslProfile directly overwrite
vscode.workspace.getConfiguration and never restore it, risking test pollution;
update the describe("getWslProfile") block to save the original getConfiguration
(e.g., const originalGetConfig = vscode.workspace.getConfiguration) in a
beforeEach and restore it in an afterEach (or use jest.spyOn(vscode.workspace,
"getConfiguration") and call mockRestore() in afterEach) so each it(...) can
safely mock getConfiguration without leaking state across tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dff403bf-4ce8-4795-9eca-970a7a523af7
📒 Files selected for processing (13)
src/__mocks__/vscode.jssrc/integrations/terminal/ExecaTerminalProcess.tssrc/integrations/terminal/Terminal.tssrc/integrations/terminal/TerminalProcess.tssrc/integrations/terminal/__tests__/ExecaTerminalProcess.spec.tssrc/integrations/terminal/__tests__/TerminalProcess.spec.tssrc/integrations/terminal/__tests__/TerminalProcess.test.tssrc/integrations/terminal/__tests__/TerminalProcessExec.bash.spec.tssrc/integrations/terminal/__tests__/TerminalProcessExec.cmd.spec.tssrc/integrations/terminal/__tests__/TerminalProcessExec.pwsh.spec.tssrc/integrations/terminal/__tests__/TerminalRegistry.spec.tssrc/utils/__tests__/shell.spec.tssrc/utils/shell.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/integrations/terminal/tests/TerminalRegistry.spec.ts
…-Org#321) Replace manual VS Code config parsing and allowlist validation in shell.ts with vscode.env.shell, the API VS Code provides for extension shell detection since version 1.37. - shell.ts: remove SHELL_ALLOWLIST (~95 entries) and manual config parsing functions (getWindowsShellFromVSCode, getMacShellFromVSCode, getLinuxShellFromVSCode). getShell() reads vscode.env.shell directly, falling back through os.userInfo(), COMSPEC/SHELL environment variables, and platform defaults. Remove getShellFallbackOccurred(). - Terminal.ts: construct terminal without explicit shellPath when the user has not set execaShellPath or configured WSL, letting VS Code's profile system determine the shell. When execaShellPath is set, pass it explicitly. For WSL, omit shellPath so VS Code uses its WSL profile for shell integration. Start the shell-integration-ready promise in the constructor. - TerminalProcess.ts: remove the getShellFallbackOccurred check that previously guarded against mismatch between the terminal's shell and the detected shell. After the shell.ts refactoring, both paths use the same VS Code resolution. When shell integration is unavailable, emit no_shell_integration without calling sendText first. In the stream processing path, treat missing OSC 633/133 markers as complete output instead of an error. - ExecaTerminalProcess.ts: use BaseTerminal.getExecaShellPath() || getShell() to match Terminal's shell resolution. When the shell is wsl.exe, spawn it directly with array arguments instead of wrapping through a shell. - tests: update shell.spec.ts for the new getShell() fallback chain. Update terminal test files to mock vscode.env.shell instead of stubbing getShell(). Extend vscode.js mock with onDidChangeTerminalShellIntegration event and env.shell. Update TerminalProcess.spec.ts to verify sendText is not called before no_shell_integration (preventing double execution). Update TerminalRegistry.spec.ts to drop the getShell spy and shellPath assertions now that Terminal no longer passes an explicit shellPath by default. Add WSL direct-spawn test in ExecaTerminalProcess.spec.ts. Fixes Zoo-Code-Org#321
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/__tests__/shell.spec.ts`:
- Line 3: The test currently imports existsSync as a named import so
vi.mocked(existsSync) does not actually stub the fs call used by getShell;
change the import to import * as fs from "fs" (so the module object is
available) and replace vi.mocked(existsSync).mockReturnValue(false) with
vi.spyOn(fs, "existsSync").mockReturnValue(false) (or alternatively call
vi.mock("fs") before imports and mock the module), ensuring the stubbed
existsSync is used by getShell in the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e0ae9fbc-7102-48cc-a5e5-957d641e1999
📒 Files selected for processing (2)
src/utils/__tests__/shell.spec.tssrc/utils/shell.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utils/shell.ts
…oo Code (Zoo-Code-Org#321) - shell.ts: remove existsSync import and revert getWslProfile() to return null when no default profile is configured. Upstream PR Zoo-Code-Org#239's PowerShell fallback was intended for the now-deleted getWindowsShellFromVSCode() and was incorrectly auto-merged into getWslProfile(), causing TS2339 errors. - shell.spec.ts: remove existsSync import and vi.mocked(existsSync) mock that were also carried in by the upstream merge. - Terminal.ts, TerminalProcess.ts, TerminalRegistry.spec.ts: rename "Roo Code" terminal name and "Roo/PS Workaround" string to Zoo Code.
3 participants
Related GitHub Issue
Closes: #321
Description
Zoo Code correctly detects the user's configured default shell (e.g., PowerShell) and tells the AI to generate commands using that shell's syntax, but the Inline Terminal actually executed commands in
cmd.exe. This caused:Write-Host) to fail with "not recognized" errorscmd.exeencodes output using the system codepage (e.g., CP936 for zh-CN) rather than UTF-8This PR connects shell detection to both terminal execution paths so the user's configured shell is used consistently.
The work evolved in two phases:
Phase 1 (original PR) passed
getShell()to both execution paths —execa({ shell: getShell() })for inline terminal andcreateTerminal({ shellPath: getShell() })for external terminal.Phase 2 (follow-up) was prompted by review feedback from @edelauna, who pointed out that passing
getShell()asshellPathon Windows with WSL configured was problematic — the function could return a Unix path (/bin/bash) that isn't valid as a WindowsshellPath. Further investigation revealed that the root issue wasn't specific to WSL: the existing shell detection inshell.tsused manual VS Code config parsing with a ~95-entry allowlist, an approach that was inherently fragile and could diverge from VS Code's own internal shell resolution.Phase 2 addressed this at the root by replacing the manual allowlist-based detection with
vscode.env.shell— the API VS Code itself uses and has provided. This eliminated the mismatch entirely and allowed several downstream simplifications.Phase 1 — Connect shell detection to execution paths
shell: truetoshell: getShell(). The detected shell is now passed to execa subprocess execution.shellPath: getShell()tovscode.window.createTerminal().shell: truetoshell: "/mock/fallback-shell".shellPathparameter.Phase 2 — Consolidate shell detection and fix WSL support
shell.ts — The largest single change. Removed
SHELL_ALLOWLIST(~95 entries),getWindowsShellFromVSCode(),getMacShellFromVSCode(),getLinuxShellFromVSCode(), andgetShellFallbackOccurred().getShell()now readsvscode.env.shelldirectly, falling back throughos.userInfo(),COMSPEC/SHELLenvironment variables, and platform defaults. Whenvscode.env.shellresolves towsl.exe, it is canonicalized toC:\Windows\System32\wsl.exeso downstream code can reliably detect WSL.Terminal.ts — Three-branch constructor replaces the single
shellPath: getShell()call: when a WSL profile is detected,shellPathis omitted so VS Code's WSL profile handles PTY bridge setup; when the user has setexecaShellPath, it is passed explicitly to honor that preference; otherwise,shellPathis omitted to defer to VS Code's profile system. TheshellIntegrationReadypromise is now started in the constructor, reducing per-command wait time.TerminalProcess.ts — Two corrections to the fallback path:
getShellFallbackOccurred()check. Both paths now derive from the same VS Code resolution; the mismatch scenario no longer exists.sendTextbeforeno_shell_integrationfallback. Previously, the code calledterminal.sendText(command)and then emittedno_shell_integration, resulting in double execution — the command ran once silently in the external terminal, then again via execa. The fallback now emitsno_shell_integrationdirectly.ExecaTerminalProcess.ts — Uses
BaseTerminal.getExecaShellPath() || getShell()for shell resolution, matching Terminal.ts. When the resolved shell iswsl.exe, spawns it directly with array arguments —wsl.exe --cd <wsl-mount-path> -- bash -c <command>— avoiding nested-quoting issues through intermediate shells. The current working directory's drive letter (e.g.,C:\) is converted to the corresponding WSL mount point (/mnt/c/).Tests (7 files updated, 1 new test added) — Updated for the new shell detection chain and WSL behavior.
Known Limitations:
Shell integration support: VS Code shell integration is only supported on a limited set of shells — Linux/macOS: bash, fish, pwsh, zsh; Windows: Git Bash, pwsh. For any shell not in this list (including WSL, cmd.exe, and custom shells), the external terminal cannot provide shell integration and will always fall back to the inline terminal after timeout. Users of unsupported shells are encouraged to enable "Use Inline Terminal" in settings.
vscode.env.shellmay not match the configured default profile for certain shells: This PR replaces the old manual allowlist-based shell detection withvscode.env.shell, which resolves via VS Code's internal terminal profile system. However, VS Code'sgetDefaultProfile()(terminalProfileService.ts:131) filters out auto-detected profiles with!e.isAutoDetected— only profiles that have been explicitly confirmed by the user (or are among the three factory defaults: PowerShell/PSCore, Command Prompt, Git Bash) can serve as the default. Profiles like "Windows PowerShell" (Windows 11 built-inpowershell.exe), "bash (MSYS2)", Cygwin, and Cmder are auto-detected but not in the factory defaults. When one of these is selected asdefaultProfile.<platform>,getDefaultProfile()returnsundefined, triggering a fallback chain:The fallback always resolves to PowerShell 7 because
enumerateDefaultPowerShellInstallations()(powershell.ts:250-305) searches PSCore first; Windows PowerShell is searched last.This is by design, not a VS Code bug. VS Code treats auto-detected paths as untrusted (they point to user-installed third-party software that may not exist or be safe) and requires explicit user confirmation via a
profiles.<platform>entry before using them as the default.Workaround: add an explicit entry for the desired shell in
terminal.integrated.profiles.<platform>with apath:This clears
isAutoDetected, allowinggetDefaultProfile()to match.Test Procedure
Unit tests:
Type checking:
Build:
Manual verification (Windows with PowerShell configured):
Prerequisites: PowerShell configured with UTF-8 encoding.
Write-Host "test"— previously failed with'Write-Host' is not recognized; now correctly outputstest.echo "中文测试"— previously produced garbled output (CP936 encoding) on zh-CN systems; now correctly outputs中文测试(UTF-8).WSL verification (Windows with WSL configured as default profile):
wsl.exespawn — no nested-quoting errors.Pre-Submission Checklist
Screenshots / Videos
N/A — backend fix with no visible UI changes.
Documentation Updates
The shell integration docs need the following updates:
"Use Inline Terminal (recommended)" section — the current text says "bypassing shell profiles and VS Code shell integration for reliability and faster starts" but doesn't specify what shell the inline terminal actually uses. Clarify: Inline Terminal now executes commands using the shell detected from VS Code's
terminal.integrated.defaultProfile.<platform>setting. Previously it always defaulted tocmd.exeon Windows (due toshell: truein execa) or/bin/shon Unix. The updated behavior means:Decision guide: Inline Terminal ON vs OFF — add a bullet about shell selection: "ON: Uses the shell from your VS Code default profile (same as OFF). Both modes now use the same detected shell."
Troubleshooting Shell Integration — add a new subsection "Shell Integration Support by Platform" listing which shells support VS Code shell integration: Linux/macOS — bash, fish, pwsh, zsh; Windows — Git Bash, pwsh. For shells outside this list (WSL, cmd.exe, MSYS2, Cygwin, Cmder), the external terminal will never receive shell integration markers and will always time out before falling back to inline terminal. Users of these shells should keep "Use Inline Terminal" ON to skip the timeout entirely.
Troubleshooting Shell Integration — add a new subsection "Garbled Command Output on Windows (Non-ASCII Locales)" covering:
cmd.exe, which encodes output using the system codepage (e.g., CP936 for zh-CN) instead of UTF-8chcpin the inline terminal — it should show code page 65001 (UTF-8). If it shows a legacy codepage (e.g., 936), configure your shell for UTF-8: for PowerShell add$OutputEncoding = [Console]::OutputEncoding = [Text.UTF8Encoding]::UTF8to your$ProfileTroubleshooting Shell Integration — add a new subsection "Default Shell Not Matching Configured Profile" covering:
terminal.integrated.defaultProfile.windows(e.g., "Windows PowerShell" or "bash (MSYS2)")getDefaultProfile()filters outisAutoDetectedprofiles; only factory defaults (PowerShell/PSCore, Command Prompt, Git Bash) and profiles with explicitpathentries pass the filter (detailed explanation in Known Limitations above)pathentry interminal.integrated.profiles.<platform>, e.g.:Additional Notes
None.
Get in Touch
Discord: Yon (yon.sinc)
Summary by CodeRabbit
New Features
Bug Fixes
Tests