Skip to content

Commit 1c791c3

Browse files
authored
feat: Improve shell detection logic for better reliability (#420)
Enhances the shell detection process by ensuring that the system's available shell is only detected if no previous shell is set. This change addresses potential issues where an undefined shell could lead to errors in subsequent operations.
1 parent 0373d15 commit 1c791c3

File tree

2 files changed

+20
-23
lines changed

2 files changed

+20
-23
lines changed

src/utils/__tests__/shell.spec.ts

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,8 @@ describe("Shell Detection Tests", () => {
157157
process.env.COMSPEC = "C:\\Windows\\System32\\cmd.exe"
158158

159159
const result = getShell()
160-
// Should return PowerShell 7 if available
161-
expect(result).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
160+
// Should return environment variable
161+
expect(result).toBe("C:\\Windows\\System32\\cmd.exe")
162162
})
163163

164164
it("uses PowerShell 7 path if source is 'PowerShell' but no explicit path", () => {
@@ -195,8 +195,7 @@ describe("Shell Detection Tests", () => {
195195
WSL: { source: "WSL" },
196196
})
197197
// Note: getShell() now prioritizes system detection over VS Code config
198-
// So it will return PowerShell 7 if available
199-
expect(getShell()).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
198+
expect(getShell()).toBe("/bin/bash")
200199
})
201200

202201
it("uses WSL bash when profile name includes 'wsl'", () => {
@@ -206,8 +205,7 @@ describe("Shell Detection Tests", () => {
206205
"Ubuntu WSL": {},
207206
})
208207
// Note: getShell() now prioritizes system detection over VS Code config
209-
// So it will return PowerShell 7 if available
210-
expect(getShell()).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
208+
expect(getShell()).toBe("/bin/bash")
211209
})
212210

213211
it("defaults to cmd.exe if no special profile is matched", () => {
@@ -216,15 +214,15 @@ describe("Shell Detection Tests", () => {
216214
mockVsCodeConfig("windows", "CommandPrompt", {
217215
CommandPrompt: {},
218216
})
219-
expect(getShell()).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
217+
expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe")
220218
})
221219

222220
it("handles undefined profile gracefully", () => {
223221
const existsSyncMock = mockExistsSync(POWER_SHELL_WHITELIST)
224222
const statSyncMock = mockStatSync(POWER_SHELL_WHITELIST)
225223
// Mock a case where defaultProfileName exists but the profile doesn't
226224
mockVsCodeConfig("windows", "NonexistentProfile", {})
227-
expect(getShell()).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
225+
expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe")
228226
})
229227

230228
it("respects userInfo() if no VS Code config is available and shell is allowed", () => {
@@ -242,7 +240,7 @@ describe("Shell Detection Tests", () => {
242240
vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any
243241
vi.mocked(userInfo).mockReturnValue({ shell: "C:\\Custom\\PowerShell.exe" } as any)
244242

245-
expect(getShell()).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
243+
expect(getShell()).toBe("C:\\Windows\\System32\\cmd.exe")
246244
})
247245

248246
it("uses COMSPEC environment variable when available", () => {
@@ -278,7 +276,7 @@ describe("Shell Detection Tests", () => {
278276

279277
// Note: getShell() now prioritizes system detection over VS Code config
280278
// So it will return the default shell for the platform
281-
expect(getShell()).toBe("/bin/zsh")
279+
expect(getShell()).toBe("/usr/local/bin/fish")
282280

283281
existsSyncMock.mockRestore()
284282
statSyncMock.mockRestore()
@@ -306,8 +304,7 @@ describe("Shell Detection Tests", () => {
306304

307305
const result = getShell()
308306
// Note: getShell() now prioritizes system detection over VS Code config
309-
// So it will return the default shell for the platform
310-
expect(result).toBe("/bin/zsh")
307+
expect(result).toBe("/opt/homebrew/bin/zsh")
311308

312309
existsSyncMock.mockRestore()
313310
statSyncMock.mockRestore()
@@ -321,7 +318,7 @@ describe("Shell Detection Tests", () => {
321318

322319
// Note: getShell() now prioritizes system detection over userInfo
323320
// So it will return the default shell for the platform
324-
expect(getShell()).toBe("/bin/zsh")
321+
expect(getShell()).toBe("/opt/homebrew/bin/zsh")
325322

326323
existsSyncMock.mockRestore()
327324
statSyncMock.mockRestore()
@@ -364,8 +361,7 @@ describe("Shell Detection Tests", () => {
364361
CustomProfile: { path: "/usr/bin/fish" },
365362
})
366363
// Note: getShell() now prioritizes system detection over VS Code config
367-
// So it will return the default shell for the platform
368-
expect(getShell()).toBe("/bin/bash")
364+
expect(getShell()).toBe("/usr/bin/fish")
369365
})
370366

371367
it("should handle array path from VSCode terminal profile", () => {
@@ -389,15 +385,15 @@ describe("Shell Detection Tests", () => {
389385
const result = getShell()
390386
// Note: getShell() now prioritizes system detection over VS Code config
391387
// So it will return the default shell for the platform
392-
expect(result).toBe("/bin/bash")
388+
expect(result).toBe("/usr/local/bin/bash")
393389
})
394390

395391
it("falls back to userInfo().shell if no VS Code config is available", () => {
396392
vscode.workspace.getConfiguration = () => ({ get: () => undefined }) as any
397393
vi.mocked(userInfo).mockReturnValue({ shell: "/usr/bin/zsh" } as any)
398394
// Note: getShell() now prioritizes system detection over userInfo
399395
// So it will return the default shell for the platform
400-
expect(getShell()).toBe("/bin/bash")
396+
expect(getShell()).toBe("/usr/bin/zsh")
401397
})
402398

403399
it("falls back to SHELL env var if no userInfo shell is found", () => {
@@ -494,8 +490,7 @@ describe("Shell Detection Tests", () => {
494490
CustomProfile: { path: "/usr/bin/fish" },
495491
})
496492
// Note: getShell() now prioritizes system detection over VS Code config
497-
// So it will return the default shell for the platform
498-
expect(getShell()).toBe("/bin/bash")
493+
expect(getShell()).toBe("/usr/bin/fish")
499494
})
500495

501496
it("should handle case-insensitive matching on Windows", () => {
@@ -507,7 +502,7 @@ describe("Shell Detection Tests", () => {
507502
})
508503
// Note: getShell() now prioritizes system detection over VS Code config
509504
// So it will return PowerShell 7 if available
510-
expect(getShell()).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
505+
expect(getShell()).toBe("c:\\windows\\system32\\cmd.exe")
511506
})
512507

513508
it("should reject unknown shells and use fallback", () => {
@@ -571,7 +566,7 @@ describe("Shell Detection Tests", () => {
571566

572567
const result = getShell()
573568
// Should return PowerShell 7 if available
574-
expect(result).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
569+
expect(result).toBe("C:\\Windows\\System32\\cmd.exe")
575570
})
576571

577572
it("should validate shells from VS Code config", () => {
@@ -619,7 +614,7 @@ describe("Shell Detection Tests", () => {
619614
const result = getShell()
620615
// Note: getShell() now prioritizes system detection over VS Code config
621616
// So it will return PowerShell 7 if available
622-
expect(result).toBe("C:\\Program Files\\PowerShell\\7\\pwsh.exe")
617+
expect(result).toBe("/bin/bash")
623618
})
624619

625620
it("should handle empty or null shell paths", () => {

src/utils/shell.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,9 @@ export function getShell(): string {
440440
shell = getShellFromUserInfo()
441441
}
442442

443-
shell = detectSystemAvailableShell()
443+
if (!shell) {
444+
shell = detectSystemAvailableShell()
445+
}
444446

445447
// 3. If still nothing, try environment variable
446448
if (!shell) {

0 commit comments

Comments
 (0)