From 1801e0a52bcb9d150c90a762e4c1ad0d8d343e91 Mon Sep 17 00:00:00 2001 From: Zoo Code Contributor Date: Tue, 26 May 2026 16:26:57 +0800 Subject: [PATCH 1/4] fix(terminal): use detected shell for execa and vscode terminal creation (#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 https://github.com/Zoo-Code-Org/Zoo-Code/issues/321 --- src/integrations/terminal/ExecaTerminalProcess.ts | 5 +++-- src/integrations/terminal/Terminal.ts | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/integrations/terminal/ExecaTerminalProcess.ts b/src/integrations/terminal/ExecaTerminalProcess.ts index cc2af93802..4f4c4f495e 100644 --- a/src/integrations/terminal/ExecaTerminalProcess.ts +++ b/src/integrations/terminal/ExecaTerminalProcess.ts @@ -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 @@ -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 @@ -111,7 +112,7 @@ export class ExecaTerminalProcess extends BaseTerminalProcess { timeoutId = setTimeout(() => { try { this.subprocess?.kill("SIGKILL") - } catch (e) {} + } catch (e) { } resolve() }, 5_000) diff --git a/src/integrations/terminal/Terminal.ts b/src/integrations/terminal/Terminal.ts index 38ace9d4b1..61b67bcab7 100644 --- a/src/integrations/terminal/Terminal.ts +++ b/src/integrations/terminal/Terminal.ts @@ -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 @@ -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() }) if (Terminal.getTerminalZdotdir()) { ShellIntegrationManager.terminalTmpDirs.set(id, env.ZDOTDIR) From 44c6e00ba77c89fa25a997c1e7839f5024e6ec57 Mon Sep 17 00:00:00 2001 From: Bo-Yu YANG Date: Tue, 26 May 2026 17:36:53 +0800 Subject: [PATCH 2/4] test(terminal): update ExecaTerminalProcess tests for getShell() integration (#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 https://github.com/Zoo-Code-Org/Zoo-Code/issues/321 Co-authored-by: Zoo Code Contributor --- .../terminal/__tests__/ExecaTerminalProcess.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts index 5f0a21869e..b7c83d205f 100644 --- a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts @@ -63,7 +63,7 @@ describe("ExecaTerminalProcess", () => { const execaMock = vitest.mocked(execa) expect(execaMock).toHaveBeenCalledWith( expect.objectContaining({ - shell: true, + shell: expect.any(String), cwd: "/test/cwd", all: true, env: expect.objectContaining({ @@ -105,13 +105,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: expect.any(String), }), ) }) From 9205427bd4fa3bd9d8fb980bd91da82258158056 Mon Sep 17 00:00:00 2001 From: Zoo Code Contributor Date: Tue, 26 May 2026 18:48:12 +0800 Subject: [PATCH 3/4] test(terminal): strengthen fallback assertions with getShell() mock Mock getShell() to verify the fallback actually uses the detected shell instead of just checking for any string value. Suggested-by: CodeRabbit --- .../terminal/__tests__/ExecaTerminalProcess.spec.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts index b7c83d205f..1b89638bfa 100644 --- a/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts +++ b/src/integrations/terminal/__tests__/ExecaTerminalProcess.spec.ts @@ -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 @@ -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, @@ -54,7 +56,7 @@ describe("ExecaTerminalProcess", () => { afterEach(() => { process.env = originalEnv - vitest.clearAllMocks() + vitest.restoreAllMocks() }) describe("UTF-8 encoding fix", () => { @@ -63,7 +65,7 @@ describe("ExecaTerminalProcess", () => { const execaMock = vitest.mocked(execa) expect(execaMock).toHaveBeenCalledWith( expect.objectContaining({ - shell: expect.any(String), + shell: "/mock/fallback-shell", cwd: "/test/cwd", all: true, env: expect.objectContaining({ @@ -111,7 +113,7 @@ describe("ExecaTerminalProcess", () => { const execaMock = vitest.mocked(execa) expect(execaMock).toHaveBeenCalledWith( expect.objectContaining({ - shell: expect.any(String), + shell: "/mock/fallback-shell", }), ) }) From 3ae0a88172d782a2d3deece9f536999d595d6b93 Mon Sep 17 00:00:00 2001 From: F915 Date: Wed, 27 May 2026 16:23:25 +0800 Subject: [PATCH 4/4] test(terminal): fix TerminalRegistry.spec.ts CI failures after getShell() integration (#321) PR #333 added shellPath and iconPath to Terminal constructor but TerminalRegistry.spec.ts assertions were not updated, causing 4 test failures on ubuntu-latest CI. --- .../__tests__/TerminalRegistry.spec.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts index 5039b2a326..cf434e0ef9 100644 --- a/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts +++ b/src/integrations/terminal/__tests__/TerminalRegistry.spec.ts @@ -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" @@ -34,6 +35,12 @@ describe("TerminalRegistry", () => { }, }) as any, ) + + vi.spyOn(shellUtils, "getShell").mockReturnValue("/mock/fallback-shell") + }) + + afterEach(() => { + vi.restoreAllMocks() }) describe("createTerminal", () => { @@ -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", }) }) @@ -64,7 +72,7 @@ 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", @@ -72,6 +80,7 @@ describe("TerminalRegistry", () => { VTE_VERSION: "0", PROMPT_EOL_MARK: "", }, + shellPath: "/mock/fallback-shell", }) } finally { // Restore original delay @@ -87,7 +96,7 @@ 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", @@ -95,6 +104,7 @@ describe("TerminalRegistry", () => { PROMPT_EOL_MARK: "", ITERM_SHELL_INTEGRATION_INSTALLED: "Yes", }, + shellPath: "/mock/fallback-shell", }) } finally { Terminal.setTerminalZshOhMy(false) @@ -109,7 +119,7 @@ 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", @@ -117,6 +127,7 @@ describe("TerminalRegistry", () => { PROMPT_EOL_MARK: "", POWERLEVEL9K_TERM_SHELL_INTEGRATION: "true", }, + shellPath: "/mock/fallback-shell", }) } finally { Terminal.setTerminalZshP10k(false)