Skip to content

fix(terminal): use detected shell for execa and vscode terminal creation (#321) #333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
F915 wants to merge 26 commits into from
Closed

fix(terminal): use detected shell for execa and vscode terminal creation (#321) #333

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
1801e0a
fix(terminal): use detected shell for execa and vscode terminal creat…
May 26, 2026
44c6e00
test(terminal): update ExecaTerminalProcess tests for getShell() inte…
F915 May 26, 2026
dc4a451
Merge branch 'main' into main
F915 May 26, 2026
9205427
test(terminal): strengthen fallback assertions with getShell() mock
May 26, 2026
e9dcb02
Merge branch 'Zoo-Code-Org:main' into main
F915 May 27, 2026
3ae0a88
test(terminal): fix TerminalRegistry.spec.ts CI failures after getShe…
F915 May 27, 2026
3ed991e
Merge branch 'Zoo-Code-Org:main' into main
F915 May 27, 2026
387ca89
fix(terminal): delegate shell detection to vscode.env.shell (#321)
F915 May 30, 2026
55f901c
Merge branch 'main' into main
F915 May 30, 2026
87cbfe7
fix(shell): resolve upstream merge conflicts and rename Roo Code to Z…
F915 May 30, 2026
b871106
Merge branch 'main' into main
F915 May 30, 2026
35ef0a6
fix(terminal): harden abort guard, add WSL UNC path conversion, and i…
F915 Jun 1, 2026
f1e065d
fix(terminal): merge upstream terminal profile override (#277) with s…
F915 Jun 3, 2026
5ffed42
fix(terminal): address code review minor issues from merge #333
F915 Jun 3, 2026
34458d3
Merge remote-tracking branch 'upstream/main'
F915 Jun 6, 2026
0a3f847
Merge upstream/main — sync to v3.58.0
F915 Jun 6, 2026
8893d0b
Merge branch 'Zoo-Code-Org:main' into main
F915 Jun 8, 2026
28b96ac
fix(terminal): harden shell detection, WSL support, and error recover…
F915 Jun 8, 2026
dcd026a
fix: harden ZDOTDIR guard, PowerShell detection, abort handling, and …
F915 Jun 8, 2026
81653a6
chore: restore .husky and .roo files accidentally dropped during upst…
F915 Jun 8, 2026
ad21186
perf(terminal): cache wslpath conversions to avoid redundant wsl.exe …
F915 Jun 9, 2026
7473874
fix(terminal): kill WSL process group from inside Linux on abort (#321)
F915 Jun 9, 2026
bc672d0
test(e2e): add shell integration regression guard with SI capability …
F915 Jun 9, 2026
b564140
fix(test): suppress require-yield lint warning in abort test generator
F915 Jun 9, 2026
1eed734
fix(terminal): clear profile override when Inline Terminal is enabled…
F915 Jun 10, 2026
b921c41
Merge branch 'main' into main
F915 Jun 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/integrations/terminal/ExecaTerminalProcess.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import process from "process"
import type { RooTerminal } from "./types"
import { BaseTerminal } from "./BaseTerminal"
import { BaseTerminalProcess } from "./BaseTerminalProcess"
import { getShell } from "../../utils/shell"

export class ExecaTerminalProcess extends BaseTerminalProcess {
private terminalRef: WeakRef<RooTerminal>
Expand Down Expand Up @@ -40,7 +41,7 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
this.isHot = true

this.subprocess = execa({
shell: BaseTerminal.getExecaShellPath() || true,
shell: BaseTerminal.getExecaShellPath() || getShell(),
cwd: this.terminal.getCurrentWorkingDirectory(),
all: true,
// Ignore stdin to ensure non-interactive mode and prevent hanging
Expand Down Expand Up @@ -111,7 +112,7 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
timeoutId = setTimeout(() => {
try {
this.subprocess?.kill("SIGKILL")
} catch (e) {}
} catch (e) { }

resolve()
}, 5_000)
Expand Down
3 changes: 2 additions & 1 deletion src/integrations/terminal/Terminal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { BaseTerminal } from "./BaseTerminal"
import { TerminalProcess } from "./TerminalProcess"
import { ShellIntegrationManager } from "./ShellIntegrationManager"
import { mergePromise } from "./mergePromise"
import { getShell } from "../../utils/shell"

export class Terminal extends BaseTerminal {
public terminal: vscode.Terminal
Expand All @@ -17,7 +18,7 @@ export class Terminal extends BaseTerminal {

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() })

@edelauna edelauna May 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@F915 F915 May 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


if (Terminal.getTerminalZdotdir()) {
ShellIntegrationManager.terminalTmpDirs.set(id, env.ZDOTDIR)
Expand Down
10 changes: 6 additions & 4 deletions src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { execa } from "execa"
import { ExecaTerminalProcess } from "../ExecaTerminalProcess"
import { BaseTerminal } from "../BaseTerminal"
import type { RooTerminal } from "../types"
import * as shellUtils from "../../../utils/shell"

describe("ExecaTerminalProcess", () => {
let mockTerminal: RooTerminal
Expand All @@ -34,6 +35,7 @@ describe("ExecaTerminalProcess", () => {
beforeEach(() => {
originalEnv = { ...process.env }
BaseTerminal.setExecaShellPath(undefined)
vitest.spyOn(shellUtils, "getShell").mockReturnValue("/mock/fallback-shell")
mockTerminal = {
provider: "execa",
id: 1,
Expand All @@ -54,7 +56,7 @@ describe("ExecaTerminalProcess", () => {

afterEach(() => {
process.env = originalEnv
vitest.clearAllMocks()
vitest.restoreAllMocks()
})

describe("UTF-8 encoding fix", () => {
Expand All @@ -63,7 +65,7 @@ describe("ExecaTerminalProcess", () => {
const execaMock = vitest.mocked(execa)
expect(execaMock).toHaveBeenCalledWith(
expect.objectContaining({
shell: true,
shell: "/mock/fallback-shell",
cwd: "/test/cwd",
all: true,
env: expect.objectContaining({
Expand Down Expand Up @@ -105,13 +107,13 @@ describe("ExecaTerminalProcess", () => {
)
})

it("should fall back to shell=true when execaShellPath is undefined", async () => {
it("should fall back to getShell() when execaShellPath is undefined", async () => {
BaseTerminal.setExecaShellPath(undefined)
await terminalProcess.run("echo test")
const execaMock = vitest.mocked(execa)
expect(execaMock).toHaveBeenCalledWith(
expect.objectContaining({
shell: true,
shell: "/mock/fallback-shell",
}),
)
})
Expand Down
19 changes: 15 additions & 4 deletions src/integrations/terminal/__tests__/TerminalRegistry.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import * as vscode from "vscode"
import { Terminal } from "../Terminal"
import { TerminalRegistry } from "../TerminalRegistry"
import * as shellUtils from "../../../utils/shell"

const PAGER = process.platform === "win32" ? "" : "cat"

Expand Down Expand Up @@ -34,6 +35,12 @@ describe("TerminalRegistry", () => {
},
}) as any,
)

vi.spyOn(shellUtils, "getShell").mockReturnValue("/mock/fallback-shell")
})

afterEach(() => {
vi.restoreAllMocks()
})

describe("createTerminal", () => {
Expand All @@ -43,13 +50,14 @@ describe("TerminalRegistry", () => {
expect(mockCreateTerminal).toHaveBeenCalledWith({
cwd: "/test/path",
name: "Roo Code",
iconPath: expect.any(Object),
iconPath: expect.objectContaining({ id: expect.any(String) }),
env: {
PAGER,
ROO_ACTIVE: "true",
VTE_VERSION: "0",
PROMPT_EOL_MARK: "",
},
shellPath: "/mock/fallback-shell",
})
})

Expand All @@ -64,14 +72,15 @@ describe("TerminalRegistry", () => {
expect(mockCreateTerminal).toHaveBeenCalledWith({
cwd: "/test/path",
name: "Roo Code",
iconPath: expect.any(Object),
iconPath: expect.objectContaining({ id: expect.any(String) }),
env: {
PAGER,
ROO_ACTIVE: "true",
PROMPT_COMMAND: "sleep 0.05",
VTE_VERSION: "0",
PROMPT_EOL_MARK: "",
},
shellPath: "/mock/fallback-shell",
})
} finally {
// Restore original delay
Expand All @@ -87,14 +96,15 @@ describe("TerminalRegistry", () => {
expect(mockCreateTerminal).toHaveBeenCalledWith({
cwd: "/test/path",
name: "Roo Code",
iconPath: expect.any(Object),
iconPath: expect.objectContaining({ id: expect.any(String) }),
env: {
PAGER,
ROO_ACTIVE: "true",
VTE_VERSION: "0",
PROMPT_EOL_MARK: "",
ITERM_SHELL_INTEGRATION_INSTALLED: "Yes",
},
shellPath: "/mock/fallback-shell",
})
} finally {
Terminal.setTerminalZshOhMy(false)
Expand All @@ -109,14 +119,15 @@ describe("TerminalRegistry", () => {
expect(mockCreateTerminal).toHaveBeenCalledWith({
cwd: "/test/path",
name: "Roo Code",
iconPath: expect.any(Object),
iconPath: expect.objectContaining({ id: expect.any(String) }),
env: {
PAGER,
ROO_ACTIVE: "true",
VTE_VERSION: "0",
PROMPT_EOL_MARK: "",
POWERLEVEL9K_TERM_SHELL_INTEGRATION: "true",
},
shellPath: "/mock/fallback-shell",
})
} finally {
Terminal.setTerminalZshP10k(false)
Expand Down
Loading