Skip to content

Commit 6a6df2c

Browse files
committed
fix(terminal): align getShell() scope with Terminal config reads
1 parent 2db2af0 commit 6a6df2c

4 files changed

Lines changed: 380 additions & 586 deletions

File tree

src/eslint-suppressions.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1776,7 +1776,7 @@
17761776
},
17771777
"utils/__tests__/shell.spec.ts": {
17781778
"@typescript-eslint/no-explicit-any": {
1779-
"count": 46
1779+
"count": 35
17801780
}
17811781
},
17821782
"utils/__tests__/storage.spec.ts": {
Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
// Regression test for https://github.com/Zoo-Code-Org/Zoo-Code/issues/634
2+
//
3+
// When the user sets terminal.integrated.defaultProfile.windows in *workspace*
4+
// settings, getShell() (used for the system prompt) picks it up via config.get()
5+
// which merges all scopes, but Terminal.getConfiguredDefaultProfileName() only
6+
// reads inspect().globalValue ?? inspect().defaultValue — intentionally excluding
7+
// workspace for security.
8+
//
9+
// The result: the system prompt tells the model "you have PowerShell" but the
10+
// actual terminal VS Code opens defaults to cmd.exe (or whatever VS Code picks
11+
// when no global/default profile is set), so every PowerShell command fails.
12+
//
13+
// Run: node_modules/.bin/vitest run integrations/terminal/__tests__/shell-system-prompt-divergence.spec.ts
14+
15+
import { existsSync } from "fs"
16+
import * as vscode from "vscode"
17+
18+
vi.mock("execa", () => ({ execa: vi.fn() }))
19+
vi.mock("fs", () => ({ existsSync: vi.fn(() => false) }))
20+
vi.mock("os", () => ({ userInfo: vi.fn(() => ({ shell: null })) }))
21+
22+
const mockedExistsSync = existsSync as unknown as ReturnType<typeof vi.fn>
23+
24+
const { Terminal } = await import("../Terminal")
25+
const { getShell } = await import("../../../utils/shell")
26+
27+
describe("issue #634 — system prompt shell vs actual terminal shell divergence", () => {
28+
let originalPlatform: NodeJS.Platform
29+
30+
beforeEach(() => {
31+
originalPlatform = process.platform
32+
Object.defineProperty(process, "platform", { value: "win32", configurable: true })
33+
Terminal.setTerminalProfile(undefined)
34+
mockedExistsSync.mockReset()
35+
// pwsh.exe exists — getShell() fallback path prefers PowerShell 7 over legacy
36+
mockedExistsSync.mockImplementation((p: string) => p === "C:\\Program Files\\PowerShell\\7\\pwsh.exe")
37+
})
38+
39+
afterEach(() => {
40+
Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true })
41+
Terminal.setTerminalProfile(undefined)
42+
vi.restoreAllMocks()
43+
})
44+
45+
/**
46+
* Stubs VS Code config to simulate a workspace-scoped default profile.
47+
*
48+
* getShell() (shell.ts) uses getConfiguration("terminal.integrated").get() which
49+
* returns the merged/effective value across all scopes — workspace value wins.
50+
*
51+
* Terminal.getConfiguredDefaultProfileName() uses inspect().globalValue ?? defaultValue,
52+
* intentionally excluding workspace scope for security. Both undefined here.
53+
*/
54+
function stubWorkspaceScopedProfile(profileName: string, profilePath: string) {
55+
const profiles = { [profileName]: { path: profilePath } }
56+
vi.spyOn(vscode.workspace, "getConfiguration").mockImplementation((section?: string) => {
57+
if (section === "terminal.integrated") {
58+
return {
59+
// get() merges all scopes — shell.ts uses this, picks up workspace value
60+
get: (key: string) => {
61+
if (key === "defaultProfile.windows") return profileName
62+
if (key === "profiles.windows") return profiles
63+
return undefined
64+
},
65+
// Terminal uses inspect() and only reads globalValue ?? defaultValue
66+
inspect: (_key: string) => ({
67+
defaultValue: undefined,
68+
globalValue: undefined,
69+
workspaceValue: profileName,
70+
}),
71+
} as unknown as vscode.WorkspaceConfiguration
72+
}
73+
74+
if (section === "terminal.integrated.profiles") {
75+
return {
76+
inspect: (_key: string) => ({
77+
defaultValue: undefined,
78+
globalValue: undefined,
79+
workspaceValue: profiles,
80+
}),
81+
} as unknown as vscode.WorkspaceConfiguration
82+
}
83+
84+
return {
85+
get: (_key: string, dv?: unknown) => dv,
86+
inspect: () => undefined,
87+
} as unknown as vscode.WorkspaceConfiguration
88+
})
89+
}
90+
91+
it("Terminal.getConfiguredDefaultProfileName ignores workspace-scoped profile (confirms the bug)", () => {
92+
// User set PowerShell as default only in their workspace .vscode/settings.json
93+
stubWorkspaceScopedProfile("PowerShell", "C:\\Program Files\\PowerShell\\7\\pwsh.exe")
94+
95+
// Terminal intentionally excludes workspace scope for security.
96+
// With no global/default profile set, it returns undefined.
97+
const terminalSeesProfileName = Terminal.getConfiguredDefaultProfileName("win32")
98+
expect(terminalSeesProfileName).toBeUndefined()
99+
100+
// As a consequence, isActiveShellPowerShell returns false even though the
101+
// user configured PowerShell — the terminal will not be treated as PowerShell.
102+
expect(Terminal.isActiveShellPowerShell("win32")).toBe(false)
103+
})
104+
105+
it("getShell() and Terminal agree on PowerShell when the default profile is set at global/user scope", () => {
106+
// When the profile is set at user (global) scope, both paths see the same value.
107+
const profilePath = "C:\\Program Files\\Git\\bin\\bash.exe" // non-PowerShell so name-matching doesn't hide the bug
108+
const profileName = "Git Bash"
109+
vi.spyOn(vscode.workspace, "getConfiguration").mockImplementation((section?: string) => {
110+
if (section === "terminal.integrated") {
111+
return {
112+
get: (key: string) => {
113+
if (key === "defaultProfile.windows") return profileName
114+
if (key === "profiles.windows") return { [profileName]: { path: profilePath } }
115+
return undefined
116+
},
117+
inspect: (_key: string) => ({
118+
defaultValue: undefined,
119+
globalValue: profileName,
120+
workspaceValue: undefined,
121+
}),
122+
} as unknown as vscode.WorkspaceConfiguration
123+
}
124+
125+
if (section === "terminal.integrated.profiles") {
126+
const profiles = { [profileName]: { path: profilePath } }
127+
return {
128+
inspect: (_key: string) => ({
129+
defaultValue: undefined,
130+
globalValue: profiles,
131+
workspaceValue: undefined,
132+
}),
133+
} as unknown as vscode.WorkspaceConfiguration
134+
}
135+
136+
return {
137+
get: (_key: string, dv?: unknown) => dv,
138+
inspect: () => undefined,
139+
} as unknown as vscode.WorkspaceConfiguration
140+
})
141+
mockedExistsSync.mockImplementation((p: string) => p === profilePath)
142+
143+
const shellForSystemPrompt = getShell()
144+
const terminalSeesProfileName = Terminal.getConfiguredDefaultProfileName("win32")
145+
146+
// Both agree: Git Bash
147+
expect(terminalSeesProfileName).toBe(profileName)
148+
expect(shellForSystemPrompt).toBe(profilePath)
149+
})
150+
151+
it("divergence: getShell() reports PowerShell but Terminal sees no profile when set at workspace scope only", () => {
152+
stubWorkspaceScopedProfile("PowerShell", "C:\\Program Files\\PowerShell\\7\\pwsh.exe")
153+
154+
// getShell() uses config.get() → picks up workspace value → sees "PowerShell" name
155+
// → name-match path returns pwsh.exe (since existsSync mocked true for it)
156+
const shellForSystemPrompt = getShell()
157+
expect(shellForSystemPrompt).toContain("PowerShell")
158+
159+
// Terminal.getConfiguredDefaultProfileName() uses inspect().globalValue → undefined
160+
const terminalSeesProfileName = Terminal.getConfiguredDefaultProfileName("win32")
161+
expect(terminalSeesProfileName).toBeUndefined()
162+
163+
// Terminal therefore can't identify the active shell as PowerShell
164+
expect(Terminal.isActiveShellPowerShell("win32")).toBe(false)
165+
166+
// Divergence: system prompt claims PowerShell, Terminal has no profile → falls
167+
// through to VS Code's own autodetect which may open cmd.exe on this machine.
168+
})
169+
})

0 commit comments

Comments
 (0)