Skip to content

Commit f0ab3eb

Browse files
committed
fix(shell): delegate VS Code profile reads to Terminal methods
1 parent 6a6df2c commit f0ab3eb

3 files changed

Lines changed: 45 additions & 37 deletions

File tree

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

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
// Regression test for https://github.com/Zoo-Code-Org/Zoo-Code/issues/634
22
//
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.
3+
// Root cause: getShell() (system prompt) used config.get() which merges all scopes
4+
// including workspace, while Terminal.getConfiguredDefaultProfileName() used
5+
// inspect().globalValue — intentionally excluding workspace scope for security.
6+
// terminal.integrated.defaultProfile.* is APPLICATION-scoped; workspace values are
7+
// technically accepted by VS Code but ignored by the terminal itself.
88
//
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.
9+
// Fix: getShell() now delegates to Terminal.getConfiguredDefaultProfileName() and
10+
// Terminal.getConfiguredProfiles(), so both paths read the same inspect()-based values
11+
// and can never disagree.
1212
//
1313
// Run: node_modules/.bin/vitest run integrations/terminal/__tests__/shell-system-prompt-divergence.spec.ts
1414

@@ -44,12 +44,9 @@ describe("issue #634 — system prompt shell vs actual terminal shell divergence
4444

4545
/**
4646
* 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.
47+
* globalValue is undefined for both the profile name and profiles map,
48+
* so Terminal (which reads only globalValue ?? defaultValue) sees no profile.
49+
* The workspace-scoped value is present to verify it is correctly ignored.
5350
*/
5451
function stubWorkspaceScopedProfile(profileName: string, profilePath: string) {
5552
const profiles = { [profileName]: { path: profilePath } }
@@ -148,22 +145,23 @@ describe("issue #634 — system prompt shell vs actual terminal shell divergence
148145
expect(shellForSystemPrompt).toBe(profilePath)
149146
})
150147

151-
it("divergence: getShell() reports PowerShell but Terminal sees no profile when set at workspace scope only", () => {
148+
it("convergence: getShell() and Terminal both ignore a workspace-scoped-only profile (fix verification)", () => {
149+
// beforeEach mocks existsSync to return true only for PS7 path.
150+
// Here we want to test the no-profile fallback, so make existsSync return false.
151+
mockedExistsSync.mockReturnValue(false)
152152
stubWorkspaceScopedProfile("PowerShell", "C:\\Program Files\\PowerShell\\7\\pwsh.exe")
153153

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
154+
// Terminal reads only inspect().globalValue → no profile configured at global scope.
160155
const terminalSeesProfileName = Terminal.getConfiguredDefaultProfileName("win32")
161156
expect(terminalSeesProfileName).toBeUndefined()
162157

163-
// Terminal therefore can't identify the active shell as PowerShell
164-
expect(Terminal.isActiveShellPowerShell("win32")).toBe(false)
158+
// After the fix, getShell() delegates to Terminal's inspect()-based methods,
159+
// so it also sees no profile. It falls back to the Windows no-profile default
160+
// (PS legacy, since existsSync returns false for PS7 in this test).
161+
const shellForSystemPrompt = getShell()
162+
expect(shellForSystemPrompt).toBe("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe")
165163

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.
164+
// Both paths agree: no profile resolved → no active shell identified as PowerShell.
165+
expect(Terminal.isActiveShellPowerShell("win32")).toBe(false)
168166
})
169167
})

src/utils/__tests__/shell.spec.ts

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,23 +36,29 @@ describe("Shell Detection Tests", () => {
3636
* how Terminal.getConfiguredDefaultProfileName and Terminal.getConfiguredProfiles
3737
* read config (globalValue only — workspace excluded per APPLICATION scope).
3838
*/
39-
function mockVsCodeConfig(_platformKey: string, defaultProfileName: string | null, profiles: Record<string, any>) {
39+
function mockVsCodeConfig(platformKey: string, defaultProfileName: string | null, profiles: Record<string, any>) {
4040
vscode.workspace.getConfiguration = (section?: string) => {
4141
if (section === "terminal.integrated") {
4242
return {
43-
inspect: (_key: string) => ({
44-
defaultValue: undefined,
45-
globalValue: defaultProfileName ?? undefined,
46-
}),
43+
inspect: (key: string) => {
44+
expect(key).toBe(`defaultProfile.${platformKey}`)
45+
return {
46+
defaultValue: undefined,
47+
globalValue: defaultProfileName ?? undefined,
48+
}
49+
},
4750
get: () => undefined,
4851
} as any
4952
}
5053
if (section === "terminal.integrated.profiles") {
5154
return {
52-
inspect: (_key: string) => ({
53-
defaultValue: undefined,
54-
globalValue: profiles,
55-
}),
55+
inspect: (key: string) => {
56+
expect(key).toBe(platformKey)
57+
return {
58+
defaultValue: undefined,
59+
globalValue: profiles,
60+
}
61+
},
5662
get: () => undefined,
5763
} as any
5864
}
@@ -450,7 +456,7 @@ describe("Shell Detection Tests", () => {
450456
expect(getShell()).toBe("/bin/bash")
451457
})
452458

453-
it("should validate array shell paths and use first allowlisted path", () => {
459+
it("should resolve array shell paths and use first path", () => {
454460
Object.defineProperty(process, "platform", { value: "win32" })
455461
vi.mocked(existsSync).mockImplementation((p: any) => p === "C:\\Program Files\\PowerShell\\7\\pwsh.exe")
456462
mockVsCodeConfig("windows", "PowerShell", {

src/utils/shell.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,10 @@ const SHELL_PATHS = {
127127
*
128128
* Returns null when no profile is configured or the profile has no resolvable path.
129129
*/
130+
function preferredWindowsPowerShell(): string {
131+
return existsSync(SHELL_PATHS.POWERSHELL_7) ? SHELL_PATHS.POWERSHELL_7 : SHELL_PATHS.POWERSHELL_LEGACY
132+
}
133+
130134
function getShellFromVSCode(): string | null {
131135
try {
132136
const profileName = Terminal.getConfiguredDefaultProfileName()
@@ -136,7 +140,7 @@ function getShellFromVSCode(): string | null {
136140
// auto-detects and prefers PowerShell 7 when installed. Mirror that so
137141
// the system prompt matches what VS Code will actually open. (issue #82)
138142
if (process.platform === "win32") {
139-
return existsSync(SHELL_PATHS.POWERSHELL_7) ? SHELL_PATHS.POWERSHELL_7 : SHELL_PATHS.POWERSHELL_LEGACY
143+
return preferredWindowsPowerShell()
140144
}
141145
return null
142146
}
@@ -156,7 +160,7 @@ function getShellFromVSCode(): string | null {
156160
// source-only PowerShell profiles (e.g. { source: "PowerShell" }) have no
157161
// path but we can still identify the shell type from the source field.
158162
if (typeof profile.source === "string" && profile.source.toLowerCase().includes("powershell")) {
159-
return existsSync(SHELL_PATHS.POWERSHELL_7) ? SHELL_PATHS.POWERSHELL_7 : SHELL_PATHS.POWERSHELL_LEGACY
163+
return preferredWindowsPowerShell()
160164
}
161165

162166
// source-only WSL profiles

0 commit comments

Comments
 (0)