Skip to content

Commit ebf4bd2

Browse files
authored
fix(prompts): report the shell that actually runs under Inline Terminal (#1682)
* fix(prompts): report the shell that actually runs under Inline Terminal getShell() now returns the shell execa actually spawns when the Inline Terminal is active: the COMSPEC path with a cmd.exe fallback on Windows and /bin/sh on POSIX, instead of the VS Code terminal profile shell. The system prompt Default Shell line therefore stops misreporting the shell that runs commands, and the shell-syntax advice derived from it matches the real execution environment. Refs #1568 * fix(prompts): seed inline-terminal default so the reported shell matches execution (Refs #1568) Align BaseTerminal's static shell-integration default with the settings default so an unseeded (headless/CLI) host reports the shell that actually executes commands; document the allowlist-gated COMSPEC divergence in getShell(); pin the divergence suite to the inline-off path and add fresh-module coverage for the seed.
1 parent 9ec139c commit ebf4bd2

4 files changed

Lines changed: 190 additions & 3 deletions

File tree

‎src/integrations/terminal/BaseTerminal.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ export abstract class BaseTerminal implements RooTerminal {
156156

157157
public static defaultShellIntegrationTimeout = 5_000
158158
private static shellIntegrationTimeout: number = BaseTerminal.defaultShellIntegrationTimeout
159-
private static shellIntegrationDisabled: boolean = false
159+
private static shellIntegrationDisabled: boolean = true
160160
private static commandDelay: number = 0
161161
private static powershellCounter: boolean = false
162162
private static terminalZshClearEolMark: boolean = true

‎src/integrations/terminal/__tests__/shell-system-prompt-divergence.spec.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ vi.mock("os", () => ({ userInfo: vi.fn(() => ({ shell: null })) }))
2121

2222
const mockedExistsSync = existsSync as unknown as ReturnType<typeof vi.fn>
2323

24+
const { BaseTerminal } = await import("../BaseTerminal")
2425
const { Terminal } = await import("../Terminal")
2526
const { getShell } = await import("../../../utils/shell")
2627

@@ -31,6 +32,9 @@ describe("issue #634 — system prompt shell vs actual terminal shell divergence
3132
originalPlatform = process.platform
3233
Object.defineProperty(process, "platform", { value: "win32", configurable: true })
3334
Terminal.setTerminalProfile(undefined)
35+
// This suite exercises the VS Code profile report path; with inline terminal
36+
// enabled, getShell() answers the execa default and ignores profiles entirely.
37+
BaseTerminal.setShellIntegrationDisabled(false)
3438
mockedExistsSync.mockReset()
3539
// pwsh.exe exists — getShell() fallback path prefers PowerShell 7 over legacy
3640
mockedExistsSync.mockImplementation((p: string) => p === "C:\\Program Files\\PowerShell\\7\\pwsh.exe")
@@ -39,6 +43,7 @@ describe("issue #634 — system prompt shell vs actual terminal shell divergence
3943
afterEach(() => {
4044
Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true })
4145
Terminal.setTerminalProfile(undefined)
46+
BaseTerminal.setShellIntegrationDisabled(false)
4247
vi.restoreAllMocks()
4348
})
4449

‎src/utils/__tests__/shell.spec.ts‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ vi.mock("path", async () => {
2828
}
2929
})
3030

31+
// Captured before any test spies patch it, so a suite can restore real profile resolution.
32+
const getProfileShellOriginal = Terminal.getProfileShell.bind(Terminal)
33+
3134
describe("Shell Detection Tests", () => {
3235
let originalPlatform: string
3336
let originalEnv: NodeJS.ProcessEnv
@@ -82,6 +85,7 @@ describe("Shell Detection Tests", () => {
8285
// Clear Zoo profile override and execa shell path between tests.
8386
Terminal.setTerminalProfile(undefined)
8487
BaseTerminal.setExecaShellPath(undefined)
88+
BaseTerminal.setShellIntegrationDisabled(false)
8589
})
8690

8791
afterEach(() => {
@@ -90,6 +94,7 @@ describe("Shell Detection Tests", () => {
9094
vscode.workspace.getConfiguration = originalGetConfig
9195
Terminal.setTerminalProfile(undefined)
9296
BaseTerminal.setExecaShellPath(undefined)
97+
BaseTerminal.setShellIntegrationDisabled(false)
9398
vi.clearAllMocks()
9499
})
95100

@@ -595,4 +600,165 @@ describe("Shell Detection Tests", () => {
595600
expect(getShell()).toBe("/bin/bash")
596601
})
597602
})
603+
604+
describe("Inline Terminal (execa default shell) — issue #1568", () => {
605+
beforeEach(() => {
606+
// Earlier suites leak a getProfileShell spy (their afterEach does not
607+
// restore spies); force the unpatched implementation in this block.
608+
vi.spyOn(Terminal, "getProfileShell").mockImplementation(getProfileShellOriginal)
609+
})
610+
611+
it("win32: reports COMSPEC cmd.exe, not the VS Code PowerShell profile", () => {
612+
Object.defineProperty(process, "platform", { value: "win32" })
613+
vi.mocked(existsSync).mockImplementation(
614+
(p) => String(p) === "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe",
615+
)
616+
mockVsCodeConfig("windows", "PowerShell", {
617+
PowerShell: { path: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe" },
618+
})
619+
process.env.COMSPEC = "C:\\Windows\\System32\\cmd.exe"
620+
BaseTerminal.setShellIntegrationDisabled(true)
621+
expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe")
622+
})
623+
624+
it("win32: without COMSPEC reports the safe cmd.exe default", () => {
625+
Object.defineProperty(process, "platform", { value: "win32" })
626+
vi.mocked(existsSync).mockReturnValue(false)
627+
mockVsCodeConfig("windows", null, {})
628+
// If the COMSPEC fallback ever resolved falsy, resolution would leak
629+
// into the userInfo step; the /bin/zsh stub makes that observable.
630+
vi.mocked(userInfo).mockReturnValue({
631+
uid: 1000,
632+
gid: 1000,
633+
username: "tester",
634+
homedir: "C:\\Users\\tester",
635+
shell: "/bin/zsh",
636+
})
637+
delete process.env.COMSPEC
638+
BaseTerminal.setShellIntegrationDisabled(true)
639+
expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe")
640+
})
641+
642+
it("win32: explicit execa shell path wins over the inline default", () => {
643+
Object.defineProperty(process, "platform", { value: "win32" })
644+
mockVsCodeConfig("windows", null, {})
645+
BaseTerminal.setExecaShellPath("C:\\Program Files\\Git\\bin\\bash.exe")
646+
BaseTerminal.setShellIntegrationDisabled(true)
647+
expect(getShell()).toBe("C:\\Program Files\\Git\\bin\\bash.exe")
648+
})
649+
650+
it("win32: non-allowlisted COMSPEC is gated to the safe cmd.exe default", () => {
651+
Object.defineProperty(process, "platform", { value: "win32" })
652+
vi.mocked(existsSync).mockReturnValue(false)
653+
mockVsCodeConfig("windows", null, {})
654+
process.env.COMSPEC = "C:\\Custom\\cmd.exe"
655+
BaseTerminal.setShellIntegrationDisabled(true)
656+
expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe")
657+
})
658+
659+
it("win32: lowercase COMSPEC passes the case-insensitive allowlist verbatim", () => {
660+
Object.defineProperty(process, "platform", { value: "win32" })
661+
vi.mocked(existsSync).mockReturnValue(false)
662+
mockVsCodeConfig("windows", null, {})
663+
process.env.COMSPEC = "c:\\windows\\system32\\cmd.exe"
664+
BaseTerminal.setShellIntegrationDisabled(true)
665+
expect(getShell()).toBe("c:\\windows\\system32\\cmd.exe")
666+
})
667+
668+
it("win32: Zoo profile override is ignored when inline is on", () => {
669+
Object.defineProperty(process, "platform", { value: "win32" })
670+
vi.mocked(existsSync).mockImplementation((p) => String(p) === "C:\\Program Files\\Git\\bin\\bash.exe")
671+
mockVsCodeConfig("windows", null, {
672+
"Git Bash": { path: "C:\\Program Files\\Git\\bin\\bash.exe" },
673+
})
674+
Terminal.setTerminalProfile("Git Bash")
675+
process.env.COMSPEC = "C:\\Windows\\System32\\cmd.exe"
676+
BaseTerminal.setShellIntegrationDisabled(true)
677+
expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe")
678+
})
679+
680+
it("darwin: reports /bin/sh even when userInfo().shell is /bin/zsh", () => {
681+
Object.defineProperty(process, "platform", { value: "darwin" })
682+
mockVsCodeConfig("osx", null, {})
683+
vi.mocked(userInfo).mockReturnValue({
684+
uid: 1000,
685+
gid: 1000,
686+
username: "tester",
687+
homedir: "/home/tester",
688+
shell: "/bin/zsh",
689+
})
690+
BaseTerminal.setShellIntegrationDisabled(true)
691+
expect(getShell()).toBe("/bin/sh")
692+
})
693+
694+
it("linux: reports /bin/sh even when SHELL env is /usr/bin/fish", () => {
695+
Object.defineProperty(process, "platform", { value: "linux" })
696+
mockVsCodeConfig("linux", null, {})
697+
process.env.SHELL = "/usr/bin/fish"
698+
BaseTerminal.setShellIntegrationDisabled(true)
699+
expect(getShell()).toBe("/bin/sh")
700+
})
701+
702+
it("win32: inline off still reports the VS Code PowerShell profile", () => {
703+
Object.defineProperty(process, "platform", { value: "win32" })
704+
vi.mocked(existsSync).mockImplementation(
705+
(p) => String(p) === "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe",
706+
)
707+
mockVsCodeConfig("windows", "PowerShell", {
708+
PowerShell: { path: "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe" },
709+
})
710+
BaseTerminal.setShellIntegrationDisabled(false)
711+
expect(getShell()).toBe("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe")
712+
})
713+
})
714+
715+
// --------------------------------------------------------------------------
716+
// Unseeded static default
717+
// --------------------------------------------------------------------------
718+
describe("Unseeded inline-terminal default", () => {
719+
// A reset module registry reproduces the headless host state: no
720+
// resolveWebviewView or updateSettings ever seeds the static, so the
721+
// declaration defaults are exactly what a fresh process starts with.
722+
it("fresh module exposes inline-terminal default matching the execution default", async () => {
723+
vi.resetModules()
724+
const { BaseTerminal: FreshBaseTerminal } = await import("../../integrations/terminal/BaseTerminal")
725+
expect(FreshBaseTerminal.getShellIntegrationDisabled()).toBe(true)
726+
})
727+
728+
it("unseeded static reports the execa shell, not the VS Code profile", async () => {
729+
vi.resetModules()
730+
Object.defineProperty(process, "platform", { value: "win32" })
731+
process.env.COMSPEC = "C:\\Windows\\System32\\cmd.exe"
732+
const psPath = "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe"
733+
// A freshly imported shell.ts resolves its own imports from the reset
734+
// registry, so the stubs below must target that generation. The profile
735+
// must actually resolve (existsSync true): a non-inline regression would
736+
// then report the profile path instead of COMSPEC and fail this test,
737+
// whereas a failed profile resolution would mask it via the COMSPEC env.
738+
const freshFs = await import("fs")
739+
vi.mocked(freshFs.existsSync).mockImplementation((p) => String(p) === psPath)
740+
// Object.assign avoids a type assertion: the fresh mock's getConfiguration
741+
// carries the generic inspect<T> signature, which a plain literal stub
742+
// cannot satisfy without widening.
743+
const freshVscode = await import("vscode")
744+
Object.assign(freshVscode.workspace, {
745+
getConfiguration: (section?: string) => ({
746+
get: () => undefined,
747+
has: () => false,
748+
inspect: (key: string) => {
749+
if (section === "terminal.integrated" && key === "defaultProfile.windows") {
750+
return { key, globalValue: "PowerShell" }
751+
}
752+
if (section === "terminal.integrated.profiles" && key === "windows") {
753+
return { key, globalValue: { PowerShell: { path: psPath } } }
754+
}
755+
return undefined
756+
},
757+
update: async () => {},
758+
}),
759+
})
760+
const { getShell: freshGetShell } = await import("../shell")
761+
expect(freshGetShell()).toBe("C:\\Windows\\System32\\cmd.exe")
762+
})
763+
})
598764
})

‎src/utils/shell.ts‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,21 @@ function getShellFromEnv(): string | null {
220220
return null
221221
}
222222

223+
/**
224+
* Returns the shell execa actually runs commands in under the Inline Terminal:
225+
* Node's spawn-shell default (COMSPEC/cmd.exe on Windows, /bin/sh on POSIX),
226+
* independent of the VS Code terminal profile (issue #1568).
227+
* Note: a COMSPEC outside the SHELL_ALLOWLIST is reported as the platform-safe
228+
* fallback by getShell()'s step-6 gate, so on such machines the reported string
229+
* and the spawned executable can differ (allowlist security tradeoff).
230+
*/
231+
function execaDefaultShellForPlatform(): string {
232+
if (process.platform === "win32") {
233+
return process.env.COMSPEC || SHELL_PATHS.CMD
234+
}
235+
return SHELL_PATHS.SH
236+
}
237+
223238
// -----------------------------------------------------
224239
// 4) Shell Validation Functions
225240
// -----------------------------------------------------
@@ -274,9 +289,10 @@ export function getShell(): string {
274289
// regardless of VS Code profile settings.
275290
shell = BaseTerminal.getExecaShellPath() ?? null
276291

277-
// 2. VS Code profile config (Zoo override first, then default profile).
292+
// 2. Inline Terminal active: execa spawns commands in its own default
293+
// shell, so the VS Code terminal profile does not run them (issue #1568).
278294
if (!shell) {
279-
shell = getShellFromVSCode()
295+
shell = BaseTerminal.getShellIntegrationDisabled() ? execaDefaultShellForPlatform() : getShellFromVSCode()
280296
}
281297

282298
// 3. If no shell from VS Code, try userInfo()

0 commit comments

Comments
 (0)