mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 08:03:28 +00:00
fix(windows): resolve the live PATH spelling by block order, not casing
Win32 resolves a duplicated variable by taking the first case-insensitive match in the block, so `resolvePathEnvKey`'s hardcoded `Path`-first preference targeted the shadowed spelling on the reporter's own `["PATH","Path"]` block. Drop the attribution-side collapse with it: it deleted the other spelling's value, and deleting the live key promotes the shadowed one, so an env that stripped down to empty lost both.
This commit is contained in:
@@ -48,22 +48,54 @@ describe('applyTerminalAttributionEnv PATH key casing', () => {
|
||||
expect(pathEntries).toContain('C:\\Program Files\\Git\\cmd')
|
||||
})
|
||||
|
||||
it('collapses a dual-cased Windows env onto the OS-resolved `Path`', () => {
|
||||
const root = makeTmpRoot()
|
||||
const baseEnv: Record<string, string> = {
|
||||
Path: 'C:\\Windows\\system32',
|
||||
PATH: 'C:\\Stale'
|
||||
// Why: Win32 resolves a duplicated variable by taking the first match in the block, so the
|
||||
// shim must land on the first-listed spelling and the shadowed one must keep its value —
|
||||
// dropping it would promote it the moment anything downstream removes the live key.
|
||||
it.each([
|
||||
{ first: 'PATH', second: 'Path' },
|
||||
{ first: 'Path', second: 'PATH' }
|
||||
])(
|
||||
'prepends the shim onto the first-listed `$first` and leaves `$second` untouched',
|
||||
({ first, second }) => {
|
||||
const root = makeTmpRoot()
|
||||
const userDataPath = join(root, 'user-data')
|
||||
const baseEnv: Record<string, string> = {
|
||||
[first]: 'C:\\tools\\bin;C:\\Windows\\system32',
|
||||
[second]: 'C:\\Windows\\system32'
|
||||
}
|
||||
|
||||
applyTerminalAttributionEnv(baseEnv, {
|
||||
enabled: true,
|
||||
platform: 'win32',
|
||||
shellFamily: 'native-windows',
|
||||
userDataPath
|
||||
})
|
||||
|
||||
const win32Dir = join(userDataPath, 'orca-terminal-attribution', 'win32')
|
||||
expect(baseEnv[first].split(';')).toEqual([
|
||||
win32Dir,
|
||||
'C:\\tools\\bin',
|
||||
'C:\\Windows\\system32'
|
||||
])
|
||||
expect(baseEnv[second]).toBe('C:\\Windows\\system32')
|
||||
}
|
||||
)
|
||||
|
||||
it('keeps a shadowed duplicate when the live spelling strips down to nothing', () => {
|
||||
const root = makeTmpRoot()
|
||||
const userDataPath = join(root, 'user-data')
|
||||
const shimDir = join(userDataPath, 'orca-terminal-attribution', 'win32')
|
||||
const baseEnv: Record<string, string> = { Path: shimDir, PATH: 'C:\\Windows\\system32' }
|
||||
|
||||
applyTerminalAttributionEnv(baseEnv, {
|
||||
enabled: true,
|
||||
enabled: false,
|
||||
platform: 'win32',
|
||||
shellFamily: 'native-windows',
|
||||
userDataPath: join(root, 'user-data')
|
||||
userDataPath
|
||||
})
|
||||
|
||||
expect(pathEnvKeys(baseEnv)).toEqual(['Path'])
|
||||
expect(baseEnv.Path).toContain('C:\\Windows\\system32')
|
||||
expect(pathEnvKeys(baseEnv)).toEqual(['PATH'])
|
||||
expect(baseEnv.PATH).toBe('C:\\Windows\\system32')
|
||||
})
|
||||
|
||||
it('strips shim entries from the inherited `Path` when attribution is disabled', () => {
|
||||
|
||||
@@ -105,7 +105,6 @@ export function applyTerminalAttributionEnv(
|
||||
// terminal environment instead of mutating global git/gh config or the
|
||||
// user's external shell PATH.
|
||||
baseEnv[pathKey] = [...prependDirs, cleanedBasePath].filter(Boolean).join(pathDelimiter)
|
||||
collapseWindowsPathEnvKey(baseEnv, pathKey, platform)
|
||||
baseEnv.ORCA_ENABLE_GIT_ATTRIBUTION = '1'
|
||||
baseEnv.ORCA_GIT_COMMIT_TRAILER = ORCA_GIT_COMMIT_TRAILER
|
||||
baseEnv.ORCA_GH_PR_FOOTER = ORCA_GH_FOOTER
|
||||
@@ -141,25 +140,6 @@ function clearTerminalAttributionEnv(
|
||||
} else {
|
||||
delete baseEnv[pathKey]
|
||||
}
|
||||
collapseWindowsPathEnvKey(baseEnv, pathKey, platform)
|
||||
}
|
||||
|
||||
/**
|
||||
* Drops the non-resolved PATH spelling so a Windows child inherits exactly one.
|
||||
*
|
||||
* Why: an inherited duplicate (WSL interop, third-party tools) is collapsed here rather than
|
||||
* left for the OS to pick a winner. Providers that re-merge their own block after this point
|
||||
* (the daemon) collapse again on their side, since this cannot see that merge.
|
||||
*/
|
||||
function collapseWindowsPathEnvKey(
|
||||
baseEnv: Record<string, string>,
|
||||
pathKey: 'PATH' | 'Path',
|
||||
platform: NodeJS.Platform
|
||||
): void {
|
||||
if (platform !== 'win32') {
|
||||
return
|
||||
}
|
||||
delete baseEnv[pathKey === 'Path' ? 'PATH' : 'Path']
|
||||
}
|
||||
|
||||
function stripAttributionPathEntries(pathValue: string, pathDelimiter: string): string {
|
||||
|
||||
@@ -301,19 +301,27 @@ describe('mergePersistedWindowsPath', () => {
|
||||
})
|
||||
|
||||
describe('resolvePathEnvKey', () => {
|
||||
it('prefers the Windows `Path` spelling already present on the env', () => {
|
||||
it('uses whichever Windows spelling is present on the env', () => {
|
||||
expect(resolvePathEnvKey({ Path: 'C:\\Windows' }, 'win32')).toBe('Path')
|
||||
expect(resolvePathEnvKey({ Path: 'C:\\Windows', PATH: 'C:\\Stale' }, 'win32')).toBe('Path')
|
||||
expect(resolvePathEnvKey({ PATH: 'C:\\Windows' }, 'win32')).toBe('PATH')
|
||||
})
|
||||
|
||||
it('keeps `PATH` on Windows when that is the only spelling present', () => {
|
||||
expect(resolvePathEnvKey({ PATH: 'C:\\Windows' }, 'win32')).toBe('PATH')
|
||||
// Why: Win32 returns the first case-insensitive match in the block, so a dual-cased env is
|
||||
// resolved by position; picking by casing would target the spelling the child never sees.
|
||||
it('resolves a dual-cased Windows env by block order, not casing', () => {
|
||||
expect(resolvePathEnvKey({ PATH: 'C:\\Live', Path: 'C:\\Shadowed' }, 'win32')).toBe('PATH')
|
||||
expect(resolvePathEnvKey({ Path: 'C:\\Live', PATH: 'C:\\Shadowed' }, 'win32')).toBe('Path')
|
||||
})
|
||||
|
||||
it('skips a path key explicitly set to undefined', () => {
|
||||
expect(resolvePathEnvKey({ PATH: undefined, Path: 'C:\\Windows' }, 'win32')).toBe('Path')
|
||||
})
|
||||
|
||||
it('falls back to the host block spelling for a Windows env with no path key', () => {
|
||||
// Why: a sparse patch that guesses wrong leaves the daemon's own merge holding both keys.
|
||||
expect(resolvePathEnvKey({}, 'win32', { PATH: 'C:\\Windows' })).toBe('PATH')
|
||||
expect(resolvePathEnvKey({}, 'win32', { Path: 'C:\\Windows' })).toBe('Path')
|
||||
expect(resolvePathEnvKey({}, 'win32', { Path: 'C:\\Live', PATH: 'C:\\Shadowed' })).toBe('Path')
|
||||
expect(resolvePathEnvKey({}, 'win32', {})).toBe('Path')
|
||||
})
|
||||
|
||||
|
||||
@@ -255,25 +255,28 @@ export function resolvePathEnvKey(
|
||||
if (platform !== 'win32') {
|
||||
return 'PATH'
|
||||
}
|
||||
if (env.Path !== undefined) {
|
||||
return 'Path'
|
||||
}
|
||||
if (env.PATH !== undefined) {
|
||||
return 'PATH'
|
||||
}
|
||||
// Why: the daemon receives a sparse env patch and re-merges its own block underneath it;
|
||||
// matching the block's spelling stops that merge from resurrecting the other one.
|
||||
return readHostPathEnvKey(hostEnv)
|
||||
return firstWindowsPathEnvKey(env) ?? firstWindowsPathEnvKey(hostEnv) ?? 'Path'
|
||||
}
|
||||
|
||||
/**
|
||||
* Reads the path spelling the host's own environment block carries.
|
||||
* Reads the path spelling a Windows child would actually resolve out of this block.
|
||||
*
|
||||
* Why: Node's Windows `process.env` lookups are case-insensitive, so only the own keys
|
||||
* reveal which spelling a child process will actually inherit.
|
||||
* Why: `RtlQueryEnvironmentVariable` scans the block linearly and returns the first
|
||||
* case-insensitive match, so position — not casing — decides the winner. Object key order is
|
||||
* that block order: Node enumerates `process.env` in block order and node-pty emits
|
||||
* `Object.keys(env)` verbatim without sorting.
|
||||
*/
|
||||
function readHostPathEnvKey(hostEnv: NodeJS.ProcessEnv): 'PATH' | 'Path' {
|
||||
return Object.keys(hostEnv).includes('PATH') ? 'PATH' : 'Path'
|
||||
function firstWindowsPathEnvKey(
|
||||
env: NodeJS.ProcessEnv | Record<string, string | undefined>
|
||||
): 'PATH' | 'Path' | undefined {
|
||||
for (const key of Object.keys(env)) {
|
||||
if ((key === 'PATH' || key === 'Path') && env[key] !== undefined) {
|
||||
return key
|
||||
}
|
||||
}
|
||||
return undefined
|
||||
}
|
||||
|
||||
function mergeWindowsPathSegments(
|
||||
|
||||
Reference in New Issue
Block a user