Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/eslint-suppressions.json
Original file line number Diff line number Diff line change
Expand Up @@ -1751,7 +1751,7 @@
},
"utils/__tests__/shell.spec.ts": {
"@typescript-eslint/no-explicit-any": {
"count": 46
"count": 35
}
},
"utils/__tests__/storage.spec.ts": {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
// Regression test for https://github.com/Zoo-Code-Org/Zoo-Code/issues/634
//
// Root cause: getShell() (system prompt) used config.get() which merges all scopes
// including workspace, while Terminal.getConfiguredDefaultProfileName() used
// inspect().globalValue — intentionally excluding workspace scope for security.
// terminal.integrated.defaultProfile.* is APPLICATION-scoped; workspace values are
// technically accepted by VS Code but ignored by the terminal itself.
//
// Fix: getShell() now delegates to Terminal.getConfiguredDefaultProfileName() and
// Terminal.getConfiguredProfiles(), so both paths read the same inspect()-based values
// and can never disagree.
//
// Run: node_modules/.bin/vitest run integrations/terminal/__tests__/shell-system-prompt-divergence.spec.ts
Comment thread
coderabbitai[bot] marked this conversation as resolved.

import { existsSync } from "fs"
import * as vscode from "vscode"

vi.mock("execa", () => ({ execa: vi.fn() }))
vi.mock("fs", () => ({ existsSync: vi.fn(() => false) }))
vi.mock("os", () => ({ userInfo: vi.fn(() => ({ shell: null })) }))

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

const { Terminal } = await import("../Terminal")
const { getShell } = await import("../../../utils/shell")

describe("issue #634 — system prompt shell vs actual terminal shell divergence", () => {
let originalPlatform: NodeJS.Platform

beforeEach(() => {
originalPlatform = process.platform
Object.defineProperty(process, "platform", { value: "win32", configurable: true })
Terminal.setTerminalProfile(undefined)
mockedExistsSync.mockReset()
// pwsh.exe exists — getShell() fallback path prefers PowerShell 7 over legacy
mockedExistsSync.mockImplementation((p: string) => p === "C:\\Program Files\\PowerShell\\7\\pwsh.exe")
})

afterEach(() => {
Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true })
Terminal.setTerminalProfile(undefined)
vi.restoreAllMocks()
})

/**
* Stubs VS Code config to simulate a workspace-scoped default profile.
* globalValue is undefined for both the profile name and profiles map,
* so Terminal (which reads only globalValue ?? defaultValue) sees no profile.
* The workspace-scoped value is present to verify it is correctly ignored.
*/
Comment thread
coderabbitai[bot] marked this conversation as resolved.
function stubWorkspaceScopedProfile(profileName: string, profilePath: string) {
const profiles = { [profileName]: { path: profilePath } }
vi.spyOn(vscode.workspace, "getConfiguration").mockImplementation((section?: string) => {
if (section === "terminal.integrated") {
return {
// get() merges all scopes — shell.ts uses this, picks up workspace value
get: (key: string) => {
if (key === "defaultProfile.windows") return profileName
if (key === "profiles.windows") return profiles
return undefined
},
// Terminal uses inspect() and only reads globalValue ?? defaultValue
inspect: (_key: string) => ({
defaultValue: undefined,
globalValue: undefined,
workspaceValue: profileName,
}),
} as unknown as vscode.WorkspaceConfiguration
}

if (section === "terminal.integrated.profiles") {
return {
inspect: (_key: string) => ({
defaultValue: undefined,
globalValue: undefined,
workspaceValue: profiles,
}),
} as unknown as vscode.WorkspaceConfiguration
}

return {
get: (_key: string, dv?: unknown) => dv,
inspect: () => undefined,
} as unknown as vscode.WorkspaceConfiguration
})
}

it("Terminal.getConfiguredDefaultProfileName ignores workspace-scoped profile (confirms the bug)", () => {
// User set PowerShell as default only in their workspace .vscode/settings.json
stubWorkspaceScopedProfile("PowerShell", "C:\\Program Files\\PowerShell\\7\\pwsh.exe")

// Terminal intentionally excludes workspace scope for security.
// With no global/default profile set, it returns undefined.
const terminalSeesProfileName = Terminal.getConfiguredDefaultProfileName("win32")
expect(terminalSeesProfileName).toBeUndefined()

// As a consequence, isActiveShellPowerShell returns false even though the
// user configured PowerShell — the terminal will not be treated as PowerShell.
expect(Terminal.isActiveShellPowerShell("win32")).toBe(false)
})

it("getShell() and Terminal agree on PowerShell when the default profile is set at global/user scope", () => {
// When the profile is set at user (global) scope, both paths see the same value.
const profilePath = "C:\\Program Files\\Git\\bin\\bash.exe" // non-PowerShell so name-matching doesn't hide the bug
const profileName = "Git Bash"
vi.spyOn(vscode.workspace, "getConfiguration").mockImplementation((section?: string) => {
if (section === "terminal.integrated") {
return {
get: (key: string) => {
if (key === "defaultProfile.windows") return profileName
if (key === "profiles.windows") return { [profileName]: { path: profilePath } }
return undefined
},
inspect: (_key: string) => ({
defaultValue: undefined,
globalValue: profileName,
workspaceValue: undefined,
}),
} as unknown as vscode.WorkspaceConfiguration
}

if (section === "terminal.integrated.profiles") {
const profiles = { [profileName]: { path: profilePath } }
return {
inspect: (_key: string) => ({
defaultValue: undefined,
globalValue: profiles,
workspaceValue: undefined,
}),
} as unknown as vscode.WorkspaceConfiguration
}

return {
get: (_key: string, dv?: unknown) => dv,
inspect: () => undefined,
} as unknown as vscode.WorkspaceConfiguration
})
mockedExistsSync.mockImplementation((p: string) => p === profilePath)

const shellForSystemPrompt = getShell()
const terminalSeesProfileName = Terminal.getConfiguredDefaultProfileName("win32")

// Both agree: Git Bash
expect(terminalSeesProfileName).toBe(profileName)
expect(shellForSystemPrompt).toBe(profilePath)
})

it("convergence: getShell() and Terminal both ignore a workspace-scoped-only profile (fix verification)", () => {
// beforeEach mocks existsSync to return true only for PS7 path.
// Here we want to test the no-profile fallback, so make existsSync return false.
mockedExistsSync.mockReturnValue(false)
stubWorkspaceScopedProfile("PowerShell", "C:\\Program Files\\PowerShell\\7\\pwsh.exe")

// Terminal reads only inspect().globalValue → no profile configured at global scope.
const terminalSeesProfileName = Terminal.getConfiguredDefaultProfileName("win32")
expect(terminalSeesProfileName).toBeUndefined()

// After the fix, getShell() delegates to Terminal's inspect()-based methods,
// so it also sees no profile. It falls back to the Windows no-profile default
// (PS legacy, since existsSync returns false for PS7 in this test).
const shellForSystemPrompt = getShell()
expect(shellForSystemPrompt).toBe("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe")

// Both paths agree: no profile resolved → no active shell identified as PowerShell.
expect(Terminal.isActiveShellPowerShell("win32")).toBe(false)
})
})
Loading
Loading