From 8bfa99d42fab795b842fb0ca2e953a275f13e546 Mon Sep 17 00:00:00 2001 From: Neil Date: Wed, 16 Sep 2026 15:58:32 -0700 Subject: [PATCH] fix(omp): keep empty config overrides at the default root --- .../host-env/omp-launch-environment.test.ts | 12 ++- .../pty/host-env/omp-launch-environment.ts | 7 +- .../omp-config-root-local-shell.test.mjs | 73 +++++++++++++++++++ 3 files changed, 86 insertions(+), 6 deletions(-) create mode 100644 tests/tools/omp-config-root-local-shell.test.mjs diff --git a/src/main/ipc/pty/host-env/omp-launch-environment.test.ts b/src/main/ipc/pty/host-env/omp-launch-environment.test.ts index 2c177d492b8..2eeeadc2c56 100644 --- a/src/main/ipc/pty/host-env/omp-launch-environment.test.ts +++ b/src/main/ipc/pty/host-env/omp-launch-environment.test.ts @@ -49,13 +49,19 @@ describe('OMP launch directory environment', () => { expect(overridden.XDG_DATA_HOME).toBe('/pane/data') }) - it('preserves explicit pane roots and intentionally empty values', async () => { + it('preserves explicit pane roots and intentionally empty XDG values', async () => { const env = { XDG_DATA_HOME: '/pane/data', XDG_STATE_HOME: '' } await inheritOmpLaunchEnvironment(env, { launchAgent: 'omp' }) expect(env.XDG_DATA_HOME).toBe('/pane/data') expect(env.XDG_STATE_HOME).toBe('') }) + it('keeps an explicitly empty OMP root at the default through profile fallback', async () => { + const env = { PI_CONFIG_DIR: '' } + await inheritOmpLaunchEnvironment(env, { launchAgent: 'omp' }) + expect(env.PI_CONFIG_DIR).toBe('.omp') + }) + it.each([ { isWsl: true, launchAgent: 'omp' }, { launchAgent: 'pi' }, @@ -82,10 +88,10 @@ describe('OMP launch directory environment', () => { expect(resolveLoginShellEnvironment).not.toHaveBeenCalled() }) - it('preserves an explicitly empty WSL config root in the daemon pane delta', async () => { + it('canonicalizes an explicitly empty WSL config root to the OMP default', async () => { const env = { PI_CONFIG_DIR: '' } await inheritOmpLaunchEnvironment(env, { isWsl: true, launchAgent: 'omp' }) - expect(env).toEqual({ PI_CONFIG_DIR: '', WSLENV: 'PI_CONFIG_DIR' }) + expect(env).toEqual({ PI_CONFIG_DIR: '.omp', WSLENV: 'PI_CONFIG_DIR' }) expect(resolveLoginShellEnvironment).not.toHaveBeenCalled() }) diff --git a/src/main/ipc/pty/host-env/omp-launch-environment.ts b/src/main/ipc/pty/host-env/omp-launch-environment.ts index 193f122a8c5..3ce95c29844 100644 --- a/src/main/ipc/pty/host-env/omp-launch-environment.ts +++ b/src/main/ipc/pty/host-env/omp-launch-environment.ts @@ -26,7 +26,7 @@ export async function inheritOmpLaunchEnvironment( if (keys.length > 0) { // WSL drops pane-provided config roots unless their names cross in WSLENV. for (const key of keys) { - env[key] = explicitEnv[key] + env[key] = key === 'PI_CONFIG_DIR' && explicitEnv[key] === '' ? '.omp' : explicitEnv[key] } addWslEnvKeys(env, keys) } @@ -42,10 +42,11 @@ export async function inheritOmpLaunchEnvironment( } const shellEnv = await resolveLoginShellEnvironment() for (const key of OMP_DIRECTORY_ENV_KEYS) { - // Explicit pane values, including empty values, take precedence over the login shell. + // Explicit pane values take precedence over the login shell. const value = (options.explicitEnv ?? env)[key] ?? shellEnv[key] ?? process.env[key] if (value !== undefined) { - env[key] = value + // OMP maps an empty config name to .omp; spell it out before profile defaults run. + env[key] = key === 'PI_CONFIG_DIR' && value === '' ? '.omp' : value } } } diff --git a/tests/tools/omp-config-root-local-shell.test.mjs b/tests/tools/omp-config-root-local-shell.test.mjs new file mode 100644 index 00000000000..27d9a62f0aa --- /dev/null +++ b/tests/tools/omp-config-root-local-shell.test.mjs @@ -0,0 +1,73 @@ +import { existsSync, mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { delimiter, join } from 'node:path' +import { afterEach, expect, it, vi } from 'vitest' +import { runProcess } from '../../src/shared/child-process/run-process' +import { inheritOmpLaunchEnvironment } from '../../src/main/ipc/pty/host-env/omp-launch-environment' +import { resetLoginShellEnvironmentCacheForTests } from '../../src/main/startup/login-shell-environment' +import { PiTitlebarExtensionService } from '../../src/main/pi/titlebar-extension-service' +const fixture = vi.hoisted(() => ({ home: '', shell: '' })) +vi.mock('node:os', async (original) => ({ ...(await original()), homedir: () => fixture.home })) +vi.mock('../../src/main/startup/hydrate-shell-path', () => ({ + resolveProfileLoadingShell: () => fixture.shell, + resolveProfileLoadingFallbackShell: () => null +})) +const shells = (process.platform === 'win32' ? [] : ['bash', 'zsh', 'fish']).flatMap((name) => { + const path = (process.env.PATH ?? '') + .split(delimiter) + .map((dir) => join(dir, name)) + .find(existsSync) + return path ? [{ name, path }] : [] +}) +afterEach(() => { + vi.unstubAllEnvs() + resetLoginShellEnvironmentCacheForTests() + if (fixture.home) { + rmSync(fixture.home, { recursive: true, force: true }) + } +}) +function prepare(shell) { + fixture.home = mkdtempSync(join(tmpdir(), 'omp20605-shell-root-')) + fixture.shell = shell.path + vi.stubEnv('HOME', fixture.home) + vi.stubEnv('ZDOTDIR', fixture.home) + vi.stubEnv('XDG_CONFIG_HOME', join(fixture.home, '.config')) + vi.stubEnv('PI_CONFIG_DIR', undefined) + const file = + shell.name === 'fish' + ? '.config/fish/config.fish' + : shell.name === 'zsh' + ? '.zprofile' + : '.bash_profile' + mkdirSync(join(fixture.home, '.config/fish'), { recursive: true }) + writeFileSync( + join(fixture.home, file), + shell.name === 'fish' + ? 'if not set -q PI_CONFIG_DIR; or test -z "$PI_CONFIG_DIR"\n set -gx PI_CONFIG_DIR .profile-omp\nend\n' + : 'export PI_CONFIG_DIR="${PI_CONFIG_DIR:-.profile-omp}"\n' + ) +} +for (const shell of shells) { + for (const config of [undefined, '.pane-omp', '']) { + it(`${shell.name}: local installer agrees with the launched shell, pane=${JSON.stringify(config)}`, async () => { + prepare(shell) + const env = config === undefined ? {} : { PI_CONFIG_DIR: config } + await inheritOmpLaunchEnvironment(env, { launchAgent: 'omp', explicitEnv: { ...env } }) + const service = new PiTitlebarExtensionService() + const managed = service.buildPtyEnv('root-proof', undefined, 'omp', { + configDirName: env.PI_CONFIG_DIR + }) + const result = await runProcess({ + program: shell.path, + args: ['-ilc', 'printf "ROOT=%s\\n" "$PI_CONFIG_DIR"'], + env: { ...process.env, ...env } + }) + expect(result.code).toBe(0) + const actual = result.stdout.match(/ROOT=([^\r\n]*)/)?.[1] + if (config === '') { + expect(actual).toBe('.omp') + } + expect(managed.ORCA_OMP_SOURCE_AGENT_DIR).toBe(join(fixture.home, actual || '.omp', 'agent')) + }) + } +}