Fix Windows dev terminal PATH and shell icons (#2152)

This commit is contained in:
Jinwoo Hong
2026-05-17 15:43:53 -04:00
committed by GitHub
parent a9fbcbc0f6
commit 102ca09a38
8 changed files with 269 additions and 40 deletions
+18 -1
View File
@@ -2,6 +2,7 @@
one focused file because the registration helper is stateful and each spawn-path
assertion reuses the same mocked IPC and node-pty harness. */
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { delimiter, join } from 'node:path'
const {
handleMock,
@@ -780,7 +781,23 @@ describe('registerPtyHandlers', () => {
try {
const env = await daemonSpawnAndGetEnv({ PATH: '/usr/bin' })
expect(env.ORCA_USER_DATA_PATH).toBe('/tmp/orca-user-data')
expect(env.PATH).toContain('/tmp/orca-user-data/cli/bin')
expect(env.PATH).toContain(join('/tmp/orca-user-data', 'cli', 'bin'))
} finally {
mockedApp.isPackaged = prev
}
})
it('preserves the inherited PATH when dev-mode daemon env omits PATH', async () => {
const { app } = await import('electron')
const mockedApp = app as unknown as { isPackaged: boolean }
const prev = mockedApp.isPackaged
mockedApp.isPackaged = false
try {
const env = await daemonSpawnAndGetEnv({}, undefined, undefined, {
PATH: '/system/bin'
})
expect(env.ORCA_USER_DATA_PATH).toBe('/tmp/orca-user-data')
expect(env.PATH).toBe(`${join('/tmp/orca-user-data', 'cli', 'bin')}${delimiter}/system/bin`)
} finally {
mockedApp.isPackaged = prev
}
+6 -1
View File
@@ -229,6 +229,10 @@ export type BuildPtyHostEnvOptions = {
githubAttributionEnabled: boolean
}
function readInheritedPath(baseEnv: Record<string, string>): string {
return baseEnv.PATH ?? process.env.PATH ?? process.env.Path ?? ''
}
/**
* Mutates `baseEnv` in place with all host-local PTY env vars and returns it.
*
@@ -337,11 +341,12 @@ export function buildPtyHostEnv(
if (!opts.isPackaged) {
baseEnv.ORCA_USER_DATA_PATH ??= opts.userDataPath
const devCliBin = join(opts.userDataPath, 'cli', 'bin')
const inheritedPath = readInheritedPath(baseEnv)
// Why: avoid a trailing delimiter when PATH is empty — some shells
// treat an empty segment as `.`, which would let commands resolve from
// the current working directory (a foot-gun we don't want to create
// for dev terminals).
baseEnv.PATH = baseEnv.PATH ? `${devCliBin}${delimiter}${baseEnv.PATH}` : devCliBin
baseEnv.PATH = inheritedPath ? `${devCliBin}${delimiter}${inheritedPath}` : devCliBin
}
// Why: GitHub attribution should only affect commands launched from
@@ -81,15 +81,10 @@ export default function SortableTab({
// because the slice returns a fresh object reference on each mark/clear.
const hasUnreadActivity = useAppStore((s) => s.unreadTerminalTabs[tab.id] === true)
// Why: on Windows, tabs created before the per-tab shell override landed (or
// created via the default Ctrl+T path without picking a specific shell)
// don't carry a shellOverride. We still want the tab-strip icon to reflect
// the shell actually running, so fall back to the user's configured default
// Windows shell. On mac/linux this resolves to undefined and the ShellIcon
// generic-terminal fallback renders.
const defaultWindowsShell = useAppStore((s) => s.settings?.terminalWindowsShell)
const isWindows = navigator.userAgent.includes('Windows')
const shellForIcon = tab.shellOverride ?? (isWindows ? defaultWindowsShell : undefined)
// Why: createTab stamps the shell used at creation time, so changing the
// default shell later does not repaint existing tabs as a different shell.
// Older persisted tabs without this field fall back to the generic icon.
const shellForIcon = tab.shellOverride
// Why: intentionally no transform/transition/opacity here. The PR's
// design is that tabs stay visually anchored during a drag — only the
@@ -276,6 +271,7 @@ export default function SortableTab({
// desaturating the brand colors beyond recognition.
<span
className={`mr-1 inline-flex shrink-0 ${isActive ? '' : 'opacity-70'}`}
data-shell-icon={shellForIcon ?? 'generic'}
aria-hidden
>
<ShellIcon shell={shellForIcon} size={12} />
@@ -5,7 +5,12 @@ import { useCallback, useMemo } from 'react'
import { toast } from 'sonner'
import { useShallow } from 'zustand/react/shallow'
import type { OpenFile } from '@/store/slices/editor'
import type { BrowserTab as BrowserTabState, Tab, TabGroup } from '../../../../shared/types'
import type {
BrowserTab as BrowserTabState,
Tab,
TabGroup,
TerminalTab
} from '../../../../shared/types'
import { useAppStore } from '../../store'
import { useAllWorktrees } from '../../store/selectors'
import { createUntitledMarkdownFile } from '../../lib/create-untitled-markdown'
@@ -21,18 +26,9 @@ export type GroupBrowserItem = BrowserTabState & { tabId: string }
const EMPTY_GROUPS: readonly TabGroup[] = []
const EMPTY_UNIFIED_TABS: readonly Tab[] = []
const EMPTY_BROWSER_TABS: readonly BrowserTabState[] = []
const EMPTY_TERMINAL_TABS: readonly TerminalTab[] = []
type TerminalTabItem = {
id: string
unifiedTabId: string
ptyId: null
worktreeId: string
title: string
customTitle: string | null
color: string | null
sortOrder: number
createdAt: number
}
type TerminalTabItem = TerminalTab & { unifiedTabId: string }
export function useTabGroupWorkspaceModel({
groupId,
@@ -51,6 +47,7 @@ export function useTabGroupWorkspaceModel({
// and blanks the window as soon as TabGroupPanel mounts.
groups: state.groupsByWorktree[worktreeId] ?? EMPTY_GROUPS,
unifiedTabs: state.unifiedTabsByWorktree[worktreeId] ?? EMPTY_UNIFIED_TABS,
terminalTabs: state.tabsByWorktree[worktreeId] ?? EMPTY_TERMINAL_TABS,
openFiles: state.openFiles,
browserTabs: state.browserTabsByWorktree[worktreeId] ?? EMPTY_BROWSER_TABS,
expandedPaneByTabId: state.expandedPaneByTabId
@@ -92,23 +89,36 @@ export function useTabGroupWorkspaceModel({
)
const activeItemId = group?.activeTabId ?? null
const activeTab = groupTabs.find((item) => item.id === activeItemId) ?? null
// Why: split groups render tab labels from unified tabs, but terminal shell
// identity lives on the terminal tab so icons survive default-shell changes.
const terminalTabById = useMemo(
() => new Map(worktreeState.terminalTabs.map((item) => [item.id, item])),
[worktreeState.terminalTabs]
)
const terminalTabs = useMemo<TerminalTabItem[]>(
() =>
groupTabs
.filter((item) => item.contentType === 'terminal')
.map((item) => ({
id: item.entityId,
unifiedTabId: item.id,
ptyId: null,
worktreeId,
title: item.label,
customTitle: item.customLabel ?? null,
color: item.color ?? null,
sortOrder: item.sortOrder,
createdAt: item.createdAt
})),
[groupTabs, worktreeId]
.map((item) => {
const terminalTab = terminalTabById.get(item.entityId)
return {
id: item.entityId,
unifiedTabId: item.id,
ptyId: terminalTab?.ptyId ?? null,
worktreeId,
title: item.label,
defaultTitle: terminalTab?.defaultTitle,
customTitle: item.customLabel ?? terminalTab?.customTitle ?? null,
color: item.color ?? terminalTab?.color ?? null,
sortOrder: item.sortOrder,
createdAt: item.createdAt,
generation: terminalTab?.generation,
shellOverride: terminalTab?.shellOverride,
pendingActivationSpawn: terminalTab?.pendingActivationSpawn
}
}),
[groupTabs, terminalTabById, worktreeId]
)
const editorItems = useMemo<GroupEditorItem[]>(
@@ -2,6 +2,7 @@
import { describe, it, expect, vi, beforeEach } from 'vitest'
import { buildWorktreeComparator } from '@/components/sidebar/smart-sort'
import type * as AgentStatusModule from '@/lib/agent-status'
import { getDefaultSettings } from '../../../../shared/constants'
// Mock sonner (imported by repos.ts)
vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } }))
@@ -628,6 +629,75 @@ describe('setActiveWorktree', () => {
expect(groups[0].tabOrder).toEqual([terminal.id])
})
it('stamps the Windows default shell onto new terminal tabs', () => {
const originalNavigator = globalThis.navigator
Object.defineProperty(globalThis, 'navigator', {
value: { userAgent: 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)' },
configurable: true
})
try {
const store = createTestStore()
const wt = 'repo1::/path/wt1'
seedStore(store, {
settings: { ...getDefaultSettings('/tmp'), terminalWindowsShell: 'wsl.exe' },
worktreesByRepo: {
repo1: [makeWorktree({ id: wt, repoId: 'repo1', path: '/path/wt1' })]
}
})
const terminal = store.getState().createTab(wt)
expect(terminal.shellOverride).toBe('wsl.exe')
store.setState({
settings: { ...store.getState().settings!, terminalWindowsShell: 'cmd.exe' }
})
expect(store.getState().tabsByWorktree[wt][0].shellOverride).toBe('wsl.exe')
} finally {
Object.defineProperty(globalThis, 'navigator', {
value: originalNavigator,
configurable: true
})
}
})
it('does not stamp local Windows shell icons onto SSH terminal tabs', () => {
const originalNavigator = globalThis.navigator
Object.defineProperty(globalThis, 'navigator', {
value: { userAgent: 'Mozilla/5.0 (Windows NT 10.0; Win64; x64)' },
configurable: true
})
try {
const store = createTestStore()
const wt = 'remote-repo::/path/wt1'
seedStore(store, {
repos: [
{
id: 'remote-repo',
path: '/remote/repo',
displayName: 'Remote Repo',
badgeColor: '#000',
addedAt: 0,
connectionId: 'ssh-1'
}
],
settings: { ...getDefaultSettings('/tmp'), terminalWindowsShell: 'wsl.exe' },
worktreesByRepo: {
'remote-repo': [makeWorktree({ id: wt, repoId: 'remote-repo', path: '/path/wt1' })]
}
})
const terminal = store.getState().createTab(wt, undefined, 'cmd.exe')
expect(terminal.shellOverride).toBeUndefined()
} finally {
Object.defineProperty(globalThis, 'navigator', {
value: originalNavigator,
configurable: true
})
}
})
it('publishes the first terminal and root tab group atomically', () => {
const store = createTestStore()
const wt = 'repo1::/path/wt1'
+46 -1
View File
@@ -59,6 +59,44 @@ function getFallbackTabTitle(tab: TerminalTab, index?: number): string {
)
}
function isWindowsRendererRuntime(): boolean {
return typeof navigator !== 'undefined' && navigator.userAgent.includes('Windows')
}
function resolveCreatedTabShellOverride(
explicitShellOverride: string | undefined,
defaultWindowsShell: string | undefined,
isRemoteWorktree: boolean
): string | undefined {
if (isRemoteWorktree) {
return undefined
}
if (explicitShellOverride !== undefined) {
return explicitShellOverride
}
if (isWindowsRendererRuntime()) {
return defaultWindowsShell
}
return undefined
}
function worktreeUsesRemoteConnection(
state: Pick<AppState, 'repos' | 'worktreesByRepo'>,
worktreeId: string
): boolean {
const directRepoId = getRepoIdFromWorktreeId(worktreeId)
const directRepo = state.repos.find((repo) => repo.id === directRepoId)
if (directRepo) {
return Boolean(directRepo.connectionId)
}
const worktree = Object.values(state.worktreesByRepo)
.flat()
.find((entry) => entry.id === worktreeId)
const repo = worktree ? state.repos.find((entry) => entry.id === worktree.repoId) : null
return Boolean(repo?.connectionId)
}
export type TerminalSlice = {
tabsByWorktree: Record<string, TerminalTab[]>
activeTabId: string | null
@@ -384,6 +422,13 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
const shouldActivate = options?.activate !== false
const nextOrdinal = getNextTerminalOrdinal(existing)
const defaultTitle = `Terminal ${nextOrdinal}`
const createdShellOverride = resolveCreatedTabShellOverride(
shellOverride,
s.settings?.terminalWindowsShell,
// Why: SSH PTYs ignore local Windows shell selection; persisting a
// local shell icon would mislabel a remote terminal.
worktreeUsesRemoteConnection(s, worktreeId)
)
tab = {
id,
// Why: CLI-created background sessions already own a PTY; revealing
@@ -399,7 +444,7 @@ export const createTerminalSlice: StateCreator<AppState, [], [], TerminalSlice>
color: null,
sortOrder: existing.length,
createdAt: Date.now(),
...(shellOverride !== undefined ? { shellOverride } : {}),
...(createdShellOverride !== undefined ? { shellOverride: createdShellOverride } : {}),
// Why: when Terminal.tsx's activation fallback auto-creates a tab for a
// first-visit worktree, the resulting PTY spawn is caused by the user
// clicking the worktree, not by work happening in it. Tagging the tab
+3 -4
View File
@@ -335,10 +335,9 @@ export type TerminalTab = {
createdAt: number
/** Bumped on shutdown so TerminalPane remounts with a fresh PTY. */
generation?: number
/** Why: records the shell this tab was explicitly opened with (e.g. 'wsl.exe'
* from the "+" submenu) so the PTY can re-use the same shell on reconnect
* without needing the user to interact with the tab again. Undefined means
* "use the default shell setting". */
/** Why: records the shell this tab was opened with (e.g. 'wsl.exe') so the
* PTY and tab icon stay stable even if the default shell setting changes
* later. Older persisted tabs may omit this field. */
shellOverride?: string
/** Why: when `setActiveWorktree` bumps generation on all-dead tabs to drive a
* TerminalPane remount, the fresh PTY that results is caused by navigation,
@@ -0,0 +1,87 @@
import { test, expect } from './helpers/orca-app'
import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store'
import {
execInTerminal,
waitForActivePanePtyId,
waitForActiveTerminalManager,
waitForTerminalOutput
} from './helpers/terminal'
test.describe('Windows terminal env and shell identity', () => {
test.beforeEach(async ({ orcaPage }) => {
await waitForSessionReady(orcaPage)
await waitForActiveWorktree(orcaPage)
await ensureTerminalVisible(orcaPage)
})
test('dev terminal preserves parent PATH so PATH commands resolve', async ({ orcaPage }) => {
await waitForActiveTerminalManager(orcaPage)
const ptyId = await waitForActivePanePtyId(orcaPage)
const marker = `__ORCA_E2E_NODE_PATH_${Date.now()}__`
// Why: before the dev PATH fallback, daemon-spawned PTYs could get PATH set
// to only Orca's dev CLI bin. A real terminal command catches that failure.
await execInTerminal(orcaPage, ptyId, `node -e "console.log('${marker}')"`)
await waitForTerminalOutput(orcaPage, marker, 15_000)
})
test('Windows tab icons stay pinned to the shell used at tab creation', async ({ orcaPage }) => {
test.skip(process.platform !== 'win32', 'Windows shell icons only render on Windows')
const tabIds = await orcaPage.evaluate(() => {
const store = window.__store
if (!store) {
throw new Error('Store unavailable')
}
const state = store.getState()
const worktreeId = state.activeWorktreeId
if (!worktreeId) {
throw new Error('No active worktree')
}
store.setState({
settings: { ...state.settings!, terminalWindowsShell: 'wsl.exe' }
})
const wslTab = store.getState().createTab(worktreeId, undefined, undefined, {
activate: false
})
store.setState({
settings: { ...store.getState().settings!, terminalWindowsShell: 'cmd.exe' }
})
const cmdTab = store.getState().createTab(worktreeId, undefined, undefined, {
activate: false
})
return { wslTabId: wslTab.id, cmdTabId: cmdTab.id }
})
const tabSnapshot = await orcaPage.evaluate(({ wslTabId, cmdTabId }) => {
const state = window.__store!.getState()
const tabs = Object.values(state.tabsByWorktree).flat()
return {
wslShell: tabs.find((tab) => tab.id === wslTabId)?.shellOverride,
cmdShell: tabs.find((tab) => tab.id === cmdTabId)?.shellOverride
}
}, tabIds)
expect(tabSnapshot).toEqual({
wslShell: 'wsl.exe',
cmdShell: 'cmd.exe'
})
const wslTab = orcaPage.locator(
`[data-testid="sortable-tab"][data-tab-id="${tabIds.wslTabId}"]`
)
const cmdTab = orcaPage.locator(
`[data-testid="sortable-tab"][data-tab-id="${tabIds.cmdTabId}"]`
)
await expect(wslTab).toBeVisible()
await expect(cmdTab).toBeVisible()
await expect(wslTab.locator('[data-shell-icon]')).toHaveAttribute('data-shell-icon', 'wsl.exe')
await expect(cmdTab.locator('[data-shell-icon]')).toHaveAttribute('data-shell-icon', 'cmd.exe')
})
})