Skip to content

Commit 98634ff

Browse files
Merge branch 'main' into feat/default-disabled-exa-mcp
2 parents f649b8c + 38d5ee0 commit 98634ff

4 files changed

Lines changed: 478 additions & 582 deletions

File tree

src/eslint-suppressions.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1706,7 +1706,7 @@
17061706
},
17071707
"utils/__tests__/shell.spec.ts": {
17081708
"@typescript-eslint/no-explicit-any": {
1709-
"count": 46
1709+
"count": 35
17101710
}
17111711
},
17121712
"utils/__tests__/storage.spec.ts": {
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
// Regression test for https://github.com/Zoo-Code-Org/Zoo-Code/issues/634
2+
//
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.
8+
//
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.
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+
* 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.
50+
*/
51+
function stubWorkspaceScopedProfile(profileName: string, profilePath: string) {
52+
const profiles = { [profileName]: { path: profilePath } }
53+
vi.spyOn(vscode.workspace, "getConfiguration").mockImplementation((section?: string) => {
54+
if (section === "terminal.integrated") {
55+
return {
56+
// get() merges all scopes — shell.ts uses this, picks up workspace value
57+
get: (key: string) => {
58+
if (key === "defaultProfile.windows") return profileName
59+
if (key === "profiles.windows") return profiles
60+
return undefined
61+
},
62+
// Terminal uses inspect() and only reads globalValue ?? defaultValue
63+
inspect: (_key: string) => ({
64+
defaultValue: undefined,
65+
globalValue: undefined,
66+
workspaceValue: profileName,
67+
}),
68+
} as unknown as vscode.WorkspaceConfiguration
69+
}
70+
71+
if (section === "terminal.integrated.profiles") {
72+
return {
73+
inspect: (_key: string) => ({
74+
defaultValue: undefined,
75+
globalValue: undefined,
76+
workspaceValue: profiles,
77+
}),
78+
} as unknown as vscode.WorkspaceConfiguration
79+
}
80+
81+
return {
82+
get: (_key: string, dv?: unknown) => dv,
83+
inspect: () => undefined,
84+
} as unknown as vscode.WorkspaceConfiguration
85+
})
86+
}
87+
88+
it("Terminal.getConfiguredDefaultProfileName ignores workspace-scoped profile (confirms the bug)", () => {
89+
// User set PowerShell as default only in their workspace .vscode/settings.json
90+
stubWorkspaceScopedProfile("PowerShell", "C:\\Program Files\\PowerShell\\7\\pwsh.exe")
91+
92+
// Terminal intentionally excludes workspace scope for security.
93+
// With no global/default profile set, it returns undefined.
94+
const terminalSeesProfileName = Terminal.getConfiguredDefaultProfileName("win32")
95+
expect(terminalSeesProfileName).toBeUndefined()
96+
97+
// As a consequence, isActiveShellPowerShell returns false even though the
98+
// user configured PowerShell — the terminal will not be treated as PowerShell.
99+
expect(Terminal.isActiveShellPowerShell("win32")).toBe(false)
100+
})
101+
102+
it("getShell() and Terminal agree on PowerShell when the default profile is set at global/user scope", () => {
103+
// When the profile is set at user (global) scope, both paths see the same value.
104+
const profilePath = "C:\\Program Files\\Git\\bin\\bash.exe" // non-PowerShell so name-matching doesn't hide the bug
105+
const profileName = "Git Bash"
106+
vi.spyOn(vscode.workspace, "getConfiguration").mockImplementation((section?: string) => {
107+
if (section === "terminal.integrated") {
108+
return {
109+
get: (key: string) => {
110+
if (key === "defaultProfile.windows") return profileName
111+
if (key === "profiles.windows") return { [profileName]: { path: profilePath } }
112+
return undefined
113+
},
114+
inspect: (_key: string) => ({
115+
defaultValue: undefined,
116+
globalValue: profileName,
117+
workspaceValue: undefined,
118+
}),
119+
} as unknown as vscode.WorkspaceConfiguration
120+
}
121+
122+
if (section === "terminal.integrated.profiles") {
123+
const profiles = { [profileName]: { path: profilePath } }
124+
return {
125+
inspect: (_key: string) => ({
126+
defaultValue: undefined,
127+
globalValue: profiles,
128+
workspaceValue: undefined,
129+
}),
130+
} as unknown as vscode.WorkspaceConfiguration
131+
}
132+
133+
return {
134+
get: (_key: string, dv?: unknown) => dv,
135+
inspect: () => undefined,
136+
} as unknown as vscode.WorkspaceConfiguration
137+
})
138+
mockedExistsSync.mockImplementation((p: string) => p === profilePath)
139+
140+
const shellForSystemPrompt = getShell()
141+
const terminalSeesProfileName = Terminal.getConfiguredDefaultProfileName("win32")
142+
143+
// Both agree: Git Bash
144+
expect(terminalSeesProfileName).toBe(profileName)
145+
expect(shellForSystemPrompt).toBe(profilePath)
146+
})
147+
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)
152+
stubWorkspaceScopedProfile("PowerShell", "C:\\Program Files\\PowerShell\\7\\pwsh.exe")
153+
154+
// Terminal reads only inspect().globalValue → no profile configured at global scope.
155+
const terminalSeesProfileName = Terminal.getConfiguredDefaultProfileName("win32")
156+
expect(terminalSeesProfileName).toBeUndefined()
157+
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")
163+
164+
// Both paths agree: no profile resolved → no active shell identified as PowerShell.
165+
expect(Terminal.isActiveShellPowerShell("win32")).toBe(false)
166+
})
167+
})

0 commit comments

Comments
 (0)