Fix Windows attribution shim ordering
This commit is contained in:
Neil
parent
d1b9392b49
commit
0d3b38af1d
|
|
@ -14,7 +14,7 @@ import {
|
|||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { applyTerminalAttributionEnv } from './terminal-attribution'
|
||||
import { applyTerminalAttributionEnv, resolveAttributionShellFamily } from './terminal-attribution'
|
||||
|
||||
describe('applyTerminalAttributionEnv', () => {
|
||||
let tmpRoot: string | null = null
|
||||
|
|
@ -61,6 +61,26 @@ describe('applyTerminalAttributionEnv', () => {
|
|||
})
|
||||
}
|
||||
|
||||
it('classifies Windows native and POSIX shell families for attribution shims', () => {
|
||||
expect(resolveAttributionShellFamily({ platform: 'win32', shellPath: 'powershell.exe' })).toBe(
|
||||
'native-windows'
|
||||
)
|
||||
expect(resolveAttributionShellFamily({ platform: 'win32', shellPath: 'cmd.exe' })).toBe(
|
||||
'native-windows'
|
||||
)
|
||||
expect(
|
||||
resolveAttributionShellFamily({
|
||||
platform: 'win32',
|
||||
shellPath: 'C:\\Program Files\\Git\\bin\\bash.exe'
|
||||
})
|
||||
).toBe('posix')
|
||||
expect(resolveAttributionShellFamily({ platform: 'win32', shellPath: 'wsl.exe' })).toBe('posix')
|
||||
expect(resolveAttributionShellFamily({ platform: 'win32', isWsl: true })).toBe('posix')
|
||||
expect(resolveAttributionShellFamily({ platform: 'darwin', shellPath: '/bin/zsh' })).toBe(
|
||||
undefined
|
||||
)
|
||||
})
|
||||
|
||||
it('does not amend HEAD when git commit --dry-run exits successfully', () => {
|
||||
const root = makeTmpRoot()
|
||||
const repo = join(root, 'repo')
|
||||
|
|
@ -730,6 +750,50 @@ exit 1
|
|||
expect(new Set(shimEntries).size).toBe(shimEntries.length)
|
||||
})
|
||||
|
||||
it('puts only Windows shims on PATH for native Windows shells', () => {
|
||||
const root = makeTmpRoot()
|
||||
const userDataPath = join(root, 'user-data')
|
||||
const baseEnv: Record<string, string> = { PATH: 'C:\\Git\\cmd;C:\\Windows\\System32' }
|
||||
|
||||
applyTerminalAttributionEnv(baseEnv, {
|
||||
enabled: true,
|
||||
platform: 'win32',
|
||||
shellFamily: 'native-windows',
|
||||
userDataPath
|
||||
})
|
||||
|
||||
const posixDir = join(userDataPath, 'orca-terminal-attribution', 'posix')
|
||||
const win32Dir = join(userDataPath, 'orca-terminal-attribution', 'win32')
|
||||
const pathEntries = baseEnv.PATH.split(';')
|
||||
|
||||
expect(pathEntries[0]).toBe(win32Dir)
|
||||
expect(pathEntries).not.toContain(posixDir)
|
||||
expect(baseEnv.ORCA_ATTRIBUTION_SHIM_DIR).toBeUndefined()
|
||||
expect(existsSync(join(win32Dir, 'git.cmd'))).toBe(true)
|
||||
})
|
||||
|
||||
it('keeps POSIX shims first for Windows Git Bash and WSL shells', () => {
|
||||
const root = makeTmpRoot()
|
||||
const userDataPath = join(root, 'user-data')
|
||||
const baseEnv: Record<string, string> = { PATH: 'C:\\Program Files\\Git\\cmd;C:\\Windows' }
|
||||
|
||||
applyTerminalAttributionEnv(baseEnv, {
|
||||
enabled: true,
|
||||
platform: 'win32',
|
||||
shellFamily: 'posix',
|
||||
userDataPath
|
||||
})
|
||||
|
||||
const posixDir = join(userDataPath, 'orca-terminal-attribution', 'posix')
|
||||
const win32Dir = join(userDataPath, 'orca-terminal-attribution', 'win32')
|
||||
const pathEntries = baseEnv.PATH.split(';')
|
||||
|
||||
expect(pathEntries[0]).toBe(posixDir)
|
||||
expect(pathEntries).not.toContain(win32Dir)
|
||||
expect(baseEnv.ORCA_ATTRIBUTION_SHIM_DIR).toBe(posixDir)
|
||||
expect(existsSync(join(posixDir, 'git'))).toBe(true)
|
||||
})
|
||||
|
||||
it('writes PowerShell wrappers without raw-template backslash escapes', () => {
|
||||
const root = makeTmpRoot()
|
||||
applyTerminalAttributionEnv(
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ scripts for both POSIX shells and Windows shells. Keeping the scripts adjacent
|
|||
to the env injection code makes the attribution behavior auditable as one unit
|
||||
instead of scattering generated shell fragments across files. */
|
||||
import { chmodSync, existsSync, mkdirSync, readFileSync, writeFileSync } from 'fs'
|
||||
import { join } from 'path'
|
||||
import { join, win32 as pathWin32 } from 'path'
|
||||
import { ORCA_GIT_COMMIT_TRAILER } from '../../shared/orca-attribution'
|
||||
|
||||
const ATTRIBUTION_ROOT_DIR = 'orca-terminal-attribution'
|
||||
|
|
@ -29,12 +29,39 @@ type AttributionShimPaths = {
|
|||
win32Dir: string
|
||||
}
|
||||
|
||||
export type AttributionShellFamily = 'native-windows' | 'posix'
|
||||
|
||||
export function resolveAttributionShellFamily(options: {
|
||||
platform?: NodeJS.Platform
|
||||
shellPath?: string
|
||||
isWsl?: boolean
|
||||
}): AttributionShellFamily | undefined {
|
||||
const platform = options.platform ?? process.platform
|
||||
if (platform !== 'win32') {
|
||||
return undefined
|
||||
}
|
||||
const shellName = options.shellPath?.replaceAll('\\', '/').split('/').pop()?.toLowerCase()
|
||||
if (options.isWsl || shellName === 'wsl.exe' || shellName === 'wsl') {
|
||||
return 'posix'
|
||||
}
|
||||
if (shellName === 'bash.exe' || shellName === 'sh.exe' || shellName === 'zsh.exe') {
|
||||
return 'posix'
|
||||
}
|
||||
return 'native-windows'
|
||||
}
|
||||
|
||||
export function applyTerminalAttributionEnv(
|
||||
baseEnv: Record<string, string>,
|
||||
options: { enabled: boolean; userDataPath: string }
|
||||
options: {
|
||||
enabled: boolean
|
||||
userDataPath: string
|
||||
platform?: NodeJS.Platform
|
||||
shellFamily?: AttributionShellFamily
|
||||
}
|
||||
): void {
|
||||
const platform = options.platform ?? process.platform
|
||||
if (!options.enabled) {
|
||||
clearTerminalAttributionEnv(baseEnv)
|
||||
clearTerminalAttributionEnv(baseEnv, platform)
|
||||
return
|
||||
}
|
||||
|
||||
|
|
@ -45,19 +72,20 @@ export function applyTerminalAttributionEnv(
|
|||
return
|
||||
}
|
||||
|
||||
const pathDelimiter = process.platform === 'win32' ? ';' : ':'
|
||||
const pathDelimiter = platform === 'win32' ? ';' : ':'
|
||||
const basePath = baseEnv.PATH ?? process.env.PATH ?? ''
|
||||
// Why: resolve real Windows commands before prepending shims so cmd wrappers
|
||||
// cannot recursively point ORCA_REAL_* at themselves.
|
||||
const resolvedGit =
|
||||
process.platform === 'win32' ? resolveWindowsExecutable('git', basePath) : null
|
||||
const resolvedGh = process.platform === 'win32' ? resolveWindowsExecutable('gh', basePath) : null
|
||||
const resolvedGit = platform === 'win32' ? resolveWindowsExecutable('git', basePath) : null
|
||||
const resolvedGh = platform === 'win32' ? resolveWindowsExecutable('gh', basePath) : null
|
||||
const { posixDir, win32Dir } = shimPaths
|
||||
// Why: Windows terminals may be cmd/PowerShell or Git Bash. Include both shim
|
||||
// families; native shells ignore extensionless POSIX files, Git Bash can use them.
|
||||
const prependDirs = process.platform === 'win32' ? [posixDir, win32Dir] : [posixDir]
|
||||
const shellFamily = options.shellFamily ?? (platform === 'win32' ? 'native-windows' : 'posix')
|
||||
// Why: Windows native shells can try to open extensionless POSIX shims before
|
||||
// PATHEXT reaches git.cmd, which surfaces an "Open With" dialog.
|
||||
const prependDirs =
|
||||
platform === 'win32' && shellFamily === 'native-windows' ? [win32Dir] : [posixDir]
|
||||
const prependDirKeys = new Set(
|
||||
prependDirs.map((dir) => (process.platform === 'win32' ? dir.toLowerCase() : dir))
|
||||
prependDirs.map((dir) => (platform === 'win32' ? dir.toLowerCase() : dir))
|
||||
)
|
||||
const cleanedBasePath = stripAttributionPathEntries(basePath, pathDelimiter)
|
||||
.split(pathDelimiter)
|
||||
|
|
@ -65,7 +93,7 @@ export function applyTerminalAttributionEnv(
|
|||
if (!entry) {
|
||||
return false
|
||||
}
|
||||
const key = process.platform === 'win32' ? entry.toLowerCase() : entry
|
||||
const key = platform === 'win32' ? entry.toLowerCase() : entry
|
||||
return !prependDirKeys.has(key)
|
||||
})
|
||||
.join(pathDelimiter)
|
||||
|
|
@ -79,9 +107,13 @@ export function applyTerminalAttributionEnv(
|
|||
baseEnv.ORCA_GIT_COMMIT_TRAILER = ORCA_GIT_COMMIT_TRAILER
|
||||
baseEnv.ORCA_GH_PR_FOOTER = ORCA_GH_FOOTER
|
||||
baseEnv.ORCA_GH_ISSUE_FOOTER = ORCA_GH_FOOTER
|
||||
baseEnv.ORCA_ATTRIBUTION_SHIM_DIR = posixDir
|
||||
if (shellFamily === 'posix') {
|
||||
baseEnv.ORCA_ATTRIBUTION_SHIM_DIR = posixDir
|
||||
} else {
|
||||
delete baseEnv.ORCA_ATTRIBUTION_SHIM_DIR
|
||||
}
|
||||
|
||||
if (process.platform === 'win32') {
|
||||
if (platform === 'win32') {
|
||||
if (resolvedGit) {
|
||||
baseEnv.ORCA_REAL_GIT = resolvedGit
|
||||
}
|
||||
|
|
@ -91,11 +123,14 @@ export function applyTerminalAttributionEnv(
|
|||
}
|
||||
}
|
||||
|
||||
function clearTerminalAttributionEnv(baseEnv: Record<string, string>): void {
|
||||
function clearTerminalAttributionEnv(
|
||||
baseEnv: Record<string, string>,
|
||||
platform: NodeJS.Platform
|
||||
): void {
|
||||
for (const key of ATTRIBUTION_ENV_KEYS) {
|
||||
delete baseEnv[key]
|
||||
}
|
||||
const pathDelimiter = process.platform === 'win32' ? ';' : ':'
|
||||
const pathDelimiter = platform === 'win32' ? ';' : ':'
|
||||
const cleanedPath = stripAttributionPathEntries(baseEnv.PATH ?? '', pathDelimiter)
|
||||
if (cleanedPath) {
|
||||
baseEnv.PATH = cleanedPath
|
||||
|
|
@ -167,12 +202,12 @@ function resolveWindowsExecutable(command: string, pathValue: string): string |
|
|||
|
||||
for (const dir of searchDirs) {
|
||||
for (const ext of pathExt) {
|
||||
const candidate = join(dir, `${command}${ext}`)
|
||||
const candidate = pathWin32.join(dir, `${command}${ext}`)
|
||||
if (existsSync(candidate)) {
|
||||
return candidate
|
||||
}
|
||||
}
|
||||
const bareCandidate = join(dir, command)
|
||||
const bareCandidate = pathWin32.join(dir, command)
|
||||
if (existsSync(bareCandidate)) {
|
||||
return bareCandidate
|
||||
}
|
||||
|
|
|
|||
|
|
@ -31,7 +31,10 @@ import {
|
|||
markClaudePtyExited,
|
||||
markClaudePtySpawned
|
||||
} from '../claude-accounts/live-pty-gate'
|
||||
import { applyTerminalAttributionEnv } from '../attribution/terminal-attribution'
|
||||
import {
|
||||
applyTerminalAttributionEnv,
|
||||
resolveAttributionShellFamily
|
||||
} from '../attribution/terminal-attribution'
|
||||
import { registerPty, unregisterPty } from '../memory/pty-registry'
|
||||
import { advertisedUrlWatcher } from '../ports/advertised-url-watcher'
|
||||
import { track } from '../telemetry/client'
|
||||
|
|
@ -303,6 +306,8 @@ export type BuildPtyHostEnvOptions = {
|
|||
* resolve to Pi for back-compat. NEVER infer from disk presence; that's
|
||||
* the bug this option fixes (cross-agent shadowing when both dirs exist). */
|
||||
launchCommand?: string
|
||||
shellPath?: string
|
||||
isWsl?: boolean
|
||||
agentStatusHooksEnabled: boolean
|
||||
networkProxySettings?: NetworkProxySettings
|
||||
}
|
||||
|
|
@ -672,7 +677,11 @@ export function buildPtyHostEnv(
|
|||
}
|
||||
applyTerminalAttributionEnv(baseEnv, {
|
||||
enabled: opts.githubAttributionEnabled,
|
||||
userDataPath: opts.userDataPath
|
||||
userDataPath: opts.userDataPath,
|
||||
shellFamily: resolveAttributionShellFamily({
|
||||
shellPath: opts.shellPath,
|
||||
isWsl: opts.isWsl
|
||||
})
|
||||
})
|
||||
|
||||
return baseEnv
|
||||
|
|
@ -922,6 +931,8 @@ export function registerPtyHandlers(
|
|||
skipCodexHomeEnv: ctx?.isWsl === true && !selectedCodexHomePath,
|
||||
githubAttributionEnabled: getSettings?.()?.enableGitHubAttribution ?? false,
|
||||
launchCommand: ctx?.command,
|
||||
shellPath: ctx?.shellPath,
|
||||
isWsl: ctx?.isWsl,
|
||||
agentStatusHooksEnabled: isAgentStatusHooksEnabled(getSettings?.()),
|
||||
networkProxySettings: getSettings?.()
|
||||
})
|
||||
|
|
@ -1453,6 +1464,8 @@ export function registerPtyHandlers(
|
|||
skipCodexHomeEnv,
|
||||
githubAttributionEnabled: getSettings?.()?.enableGitHubAttribution ?? false,
|
||||
launchCommand: args.command,
|
||||
shellPath: daemonShellOverride ?? process.env.COMSPEC,
|
||||
isWsl: shouldSkipCodexHomeEnvForWindowsShell(daemonShellOverride, args.cwd),
|
||||
agentStatusHooksEnabled: isAgentStatusHooksEnabled(getSettings?.()),
|
||||
networkProxySettings: getSettings?.()
|
||||
})
|
||||
|
|
@ -1898,6 +1911,8 @@ export function registerPtyHandlers(
|
|||
skipCodexHomeEnv,
|
||||
githubAttributionEnabled: getSettings?.()?.enableGitHubAttribution ?? false,
|
||||
launchCommand: args.command,
|
||||
shellPath: effectiveShellOverride ?? process.env.COMSPEC,
|
||||
isWsl: shouldSkipCodexHomeEnvForWindowsShell(effectiveShellOverride, args.cwd),
|
||||
agentStatusHooksEnabled: isAgentStatusHooksEnabled(getSettings?.()),
|
||||
networkProxySettings: getSettings?.()
|
||||
})
|
||||
|
|
|
|||
|
|
@ -162,7 +162,7 @@ export type LocalPtyProviderOptions = {
|
|||
buildSpawnEnv?: (
|
||||
id: string,
|
||||
baseEnv: Record<string, string>,
|
||||
ctx?: { command?: string; isWsl?: boolean; wslDistro?: string | null }
|
||||
ctx?: { command?: string; shellPath?: string; isWsl?: boolean; wslDistro?: string | null }
|
||||
) => Record<string, string>
|
||||
/** Whether worktree-scoped shell history is enabled. When true (or absent)
|
||||
* and a worktreeId is provided, HISTFILE is scoped per-worktree. */
|
||||
|
|
@ -332,6 +332,7 @@ export class LocalPtyProvider implements IPtyProvider {
|
|||
const finalEnv = this.opts.buildSpawnEnv
|
||||
? this.opts.buildSpawnEnv(id, spawnEnv, {
|
||||
command: args.command,
|
||||
shellPath,
|
||||
isWsl: isWslShell,
|
||||
wslDistro: launchWslDistro
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in New Issue