Skip to content

Commit bac8adc

Browse files
authored
fix(terminal): prevent inline terminal cmd.exe fallback on Windows (#1673)
* fix(terminal): prevent inline terminal cmd.exe fallback on Windows BaseTerminalProcess.execaOptions previously passed shell: BaseTerminal.getExecaShellPath() || true, so an unset execaShellPath fell back to shell:true. On that branch the shell process becomes a bare cmd.exe instead of the resolved PowerShell/Zoo profile, causing the inline terminal to silently downgrade to Windows Command Prompt. Change the fallback to ?? getShell(), which resolves through VS Code profile config -> Zoo override -> userInfo -> env -> allowlisted default (never shell:true). Explicit execaShellPath still wins verbatim, and a deliberately selected cmd.exe profile is preserved via getShell(). Adds a cross-path regression suite covering explicit-win, unset->getShell(), PowerShell-via-configured-profiles, deliberate-Command-Prompt preservation, and never-shell:true. * Fix empty execa shell path fallback Use a truthy fallback so an empty persisted execaShellPath resolves through getShell() instead of being passed through as an empty shell value. * Tighten shell path assertion Assert the exact mocked PowerShell path instead of matching only the executable name.
1 parent 7c302a5 commit bac8adc

2 files changed

Lines changed: 60 additions & 4 deletions

File tree

src/integrations/terminal/ExecaTerminalProcess.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import process from "process"
55
import type { RooTerminal } from "./types"
66
import { BaseTerminal } from "./BaseTerminal"
77
import { BaseTerminalProcess } from "./BaseTerminalProcess"
8+
import { getShell } from "../../utils/shell"
89

910
export class ExecaTerminalProcess extends BaseTerminalProcess {
1011
private terminalRef: WeakRef<RooTerminal>
@@ -40,7 +41,7 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
4041
this.isHot = true
4142

4243
this.subprocess = execa({
43-
shell: BaseTerminal.getExecaShellPath() || true,
44+
shell: BaseTerminal.getExecaShellPath() || getShell(),
4445
cwd: this.terminal.getCurrentWorkingDirectory(),
4546
all: true,
4647
// Ignore stdin to ensure non-interactive mode and prevent hanging

src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ vitest.mock("ps-tree", () => ({
2525

2626
import { execa } from "execa"
2727
import { ExecaTerminalProcess } from "../ExecaTerminalProcess"
28+
import * as shellUtils from "../../../utils/shell"
2829
import { BaseTerminal } from "../BaseTerminal"
2930
import type { RooTerminal } from "../types"
3031

@@ -63,11 +64,13 @@ describe("ExecaTerminalProcess", () => {
6364

6465
describe("UTF-8 encoding fix", () => {
6566
it("should set LANG and LC_ALL to en_US.UTF-8", async () => {
67+
// Deterministic shell so the assertion focuses solely on LANG/LC_ALL.
68+
vi.spyOn(shellUtils, "getShell").mockReturnValue("/bin/zsh")
6669
await terminalProcess.run("echo test")
6770
const execaMock = vitest.mocked(execa)
6871
expect(execaMock).toHaveBeenCalledWith(
6972
expect.objectContaining({
70-
shell: true,
73+
shell: "/bin/zsh",
7174
cwd: "/test/cwd",
7275
all: true,
7376
env: expect.objectContaining({
@@ -109,15 +112,19 @@ describe("ExecaTerminalProcess", () => {
109112
)
110113
})
111114

112-
it("should fall back to shell=true when execaShellPath is undefined", async () => {
115+
it("when execaShellPath is unset, Execa resolves through getShell() (never shell:true)", async () => {
113116
BaseTerminal.setExecaShellPath(undefined)
117+
const resolved = "/resolved/pwsh.exe"
118+
const getShellSpy = vi.spyOn(shellUtils, "getShell").mockReturnValue(resolved)
114119
await terminalProcess.run("echo test")
115120
const execaMock = vitest.mocked(execa)
121+
expect(getShellSpy).toHaveBeenCalledTimes(1)
116122
expect(execaMock).toHaveBeenCalledWith(
117123
expect.objectContaining({
118-
shell: true,
124+
shell: resolved,
119125
}),
120126
)
127+
expect(execaMock).not.toHaveBeenCalledWith(expect.objectContaining({ shell: true }))
121128
})
122129
})
123130

@@ -191,4 +198,52 @@ describe("ExecaTerminalProcess", () => {
191198
expect(terminalProcess["lastRetrievedIndex"]).toBe(0)
192199
})
193200
})
201+
202+
describe("cross-path shell invariant (#705 regression)", () => {
203+
// Bridge through unknown: the mock records the raw options object, whose
204+
// declared type under execa's overloads is string|URL, not a plain record.
205+
const capturedShellOption = (): Record<string, string | boolean> =>
206+
vitest.mocked(execa).mock.calls[0][0] as unknown as Record<string, string | boolean>
207+
208+
beforeEach(() => {
209+
BaseTerminal.setExecaShellPath(undefined)
210+
})
211+
212+
it("system-prompt resolved shell == Execa execution shell when no explicit execaShellPath", async () => {
213+
const getShellSpy = vi.spyOn(shellUtils, "getShell").mockReturnValue("/bin/zsh")
214+
await terminalProcess.run("echo test")
215+
expect(getShellSpy).toHaveBeenCalledTimes(1)
216+
expect(capturedShellOption().shell).toBe("/bin/zsh")
217+
})
218+
219+
it("keeps the Execa shell equal to getShell() when a Zoo profile override is set", async () => {
220+
BaseTerminal.setExecaShellPath(undefined)
221+
const getShellSpy = vi.spyOn(shellUtils, "getShell").mockReturnValue("C:\\Windows\\System32\\pwsh.exe")
222+
await terminalProcess.run("echo test")
223+
expect(getShellSpy).toHaveBeenCalledTimes(1)
224+
expect(capturedShellOption().shell).toBe("C:\\Windows\\System32\\pwsh.exe")
225+
})
226+
227+
it("uses PowerShell when VS Code resolves PowerShell and execaShellPath is unset", async () => {
228+
const getShellSpy = vi.spyOn(shellUtils, "getShell").mockReturnValue("powershell.exe")
229+
await terminalProcess.run("echo test")
230+
expect(getShellSpy).toHaveBeenCalledTimes(1)
231+
expect(capturedShellOption().shell).toBe("powershell.exe")
232+
})
233+
234+
it("preserves a deliberately selected Command Prompt profile when execaShellPath is unset", async () => {
235+
const getShellSpy = vi.spyOn(shellUtils, "getShell").mockReturnValue("cmd.exe")
236+
await terminalProcess.run("echo test")
237+
expect(getShellSpy).toHaveBeenCalledTimes(1)
238+
expect(capturedShellOption().shell).toBe("cmd.exe")
239+
})
240+
241+
it("does NOT delegate to shell:true even when getShell() returns an unusual path", async () => {
242+
const getShellSpy = vi.spyOn(shellUtils, "getShell").mockReturnValue("/opt/custom/fish")
243+
await terminalProcess.run("echo test")
244+
expect(getShellSpy).toHaveBeenCalledTimes(1)
245+
expect(capturedShellOption().shell).not.toBe(true)
246+
expect(capturedShellOption().shell).toBe("/opt/custom/fish")
247+
})
248+
})
194249
})

0 commit comments

Comments
 (0)