diff --git a/main/src/container-engine.ts b/main/src/container-engine.ts index c18390c3d..6b13cfa94 100644 --- a/main/src/container-engine.ts +++ b/main/src/container-engine.ts @@ -2,6 +2,7 @@ import { exec } from 'node:child_process' import { promisify } from 'node:util' import { platform } from 'node:os' import log from './logger' +import { createEnhancedPath } from './utils/enhanced-path' const execAsync = promisify(exec) @@ -11,45 +12,6 @@ interface ContainerEngineStatus { available: boolean } -// Common installation paths by platform -const getCommonPaths = (): string[] => { - const currentPlatform = platform() - - switch (currentPlatform) { - case 'darwin': - return [ - '/Applications/Docker.app/Contents/Resources/bin', - '/opt/homebrew/bin', - '/usr/local/bin', - '~/.rd/bin', - ] - case 'linux': - return ['/usr/local/bin', '/opt/homebrew/bin', '/snap/bin', '~/.rd/bin'] - case 'win32': - return [ - 'C:\\Program Files\\Docker\\Docker\\resources\\bin', - 'C:\\Program Files\\RedHat\\Podman', - ] - default: - return [] - } -} - -const expandPath = (path: string): string => - path.startsWith('~') - ? path.replace('~', process.env.HOME || process.env.USERPROFILE || '') - : path - -const createEnhancedPath = (): string => { - const commonPaths = getCommonPaths().map(expandPath) - const currentPath = process.env.PATH || '' - const separator = platform() === 'win32' ? ';' : ':' - - return [...commonPaths, ...currentPath.split(separator)] - .filter(Boolean) - .join(separator) -} - const tryCommand = async (command: string): Promise => { try { await execAsync(command) diff --git a/main/src/tests/toolhive-manager.test.ts b/main/src/tests/toolhive-manager.test.ts index e56ba50a3..550a43ac7 100644 --- a/main/src/tests/toolhive-manager.test.ts +++ b/main/src/tests/toolhive-manager.test.ts @@ -3,6 +3,7 @@ import { EventEmitter } from 'node:events' import { vol } from 'memfs' import { spawn } from 'node:child_process' import net from 'node:net' +import { platform } from 'node:os' import { app } from 'electron' import { startToolhive, @@ -32,6 +33,18 @@ vi.mock('node:child_process', async (importOriginal) => { }, } }) +vi.mock('node:os', async (importOriginal) => { + const actual = await importOriginal() + const platformMock = vi.fn(actual.platform) + return { + ...actual, + platform: platformMock, + default: { + ...actual, + platform: platformMock, + }, + } +}) vi.mock('node:fs') vi.mock('node:net', async (importOriginal) => { const actual = await importOriginal() @@ -454,6 +467,32 @@ describe('toolhive-manager', () => { expect(spawnArgs.some((a) => a.startsWith('--sentry-'))).toBe(false) }) + it('spawns with an enhanced PATH that includes common container-tooling dirs', async () => { + vi.mocked(platform).mockReturnValue('darwin') + vi.stubEnv('PATH', '/usr/bin:/bin') + + const startPromise = startToolhive() + await vi.advanceTimersByTimeAsync(50) + await startPromise + + const spawnOptions = mockSpawn.mock.calls[0]![2] as { + env: Record + } + const spawnedPath = spawnOptions.env.PATH + expect(spawnedPath).toBeDefined() + const entries = spawnedPath!.split(':') + + expect(entries).toContain( + '/Applications/Docker.app/Contents/Resources/bin' + ) + expect(entries).toContain('/opt/homebrew/bin') + expect(entries).toContain('/usr/local/bin') + expect(entries).toContain('/usr/bin') + expect(entries).toContain('/bin') + + expect(spawnOptions.env.TOOLHIVE_SKIP_DESKTOP_CHECK).toBe('true') + }) + it('omits sentry flags when telemetry is disabled', async () => { vi.stubEnv('VITE_SENTRY_THV_DSN', 'https://test@sentry.io/123') vi.mocked(readSetting).mockReturnValue('false') diff --git a/main/src/toolhive-manager.ts b/main/src/toolhive-manager.ts index 9c46956dd..27806b163 100644 --- a/main/src/toolhive-manager.ts +++ b/main/src/toolhive-manager.ts @@ -8,6 +8,7 @@ import log from './logger' import * as Sentry from '@sentry/electron/main' import { getQuittingState } from './app-state' import { readSetting } from './db/readers/settings-reader' +import { createEnhancedPath } from './utils/enhanced-path' import { ALREADY_RUNNING, REGISTRY_AUTH_REQUIRED, @@ -182,6 +183,7 @@ export async function startToolhive(): Promise { windowsHide: true, env: { ...process.env, + PATH: createEnhancedPath(), TOOLHIVE_SKIP_DESKTOP_CHECK: 'true', }, }) diff --git a/main/src/utils/__tests__/enhanced-path.test.ts b/main/src/utils/__tests__/enhanced-path.test.ts new file mode 100644 index 000000000..3252adc74 --- /dev/null +++ b/main/src/utils/__tests__/enhanced-path.test.ts @@ -0,0 +1,128 @@ +import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest' +import { platform } from 'node:os' + +import { createEnhancedPath } from '../enhanced-path' + +vi.mock('node:os', async (importOriginal) => { + const actual = await importOriginal() + const platformMock = vi.fn(actual.platform) + return { + ...actual, + platform: platformMock, + default: { + ...actual, + platform: platformMock, + }, + } +}) + +const mockPlatform = vi.mocked(platform) + +describe('createEnhancedPath', () => { + beforeEach(() => { + mockPlatform.mockReset() + }) + + afterEach(() => { + vi.unstubAllEnvs() + }) + + it('prepends macOS container-tooling paths on darwin', () => { + mockPlatform.mockReturnValue('darwin') + vi.stubEnv('PATH', '/usr/bin:/bin') + vi.stubEnv('HOME', '/Users/test') + + const result = createEnhancedPath() + const entries = result.split(':') + + expect(entries.slice(0, 4)).toEqual([ + '/Applications/Docker.app/Contents/Resources/bin', + '/opt/homebrew/bin', + '/usr/local/bin', + '/Users/test/.rd/bin', + ]) + expect(entries).toContain('/usr/bin') + expect(entries).toContain('/bin') + }) + + it('expands ~ using HOME', () => { + mockPlatform.mockReturnValue('darwin') + vi.stubEnv('PATH', '') + vi.stubEnv('HOME', '/Users/alice') + + const result = createEnhancedPath() + + expect(result).toContain('/Users/alice/.rd/bin') + expect(result).not.toContain('~') + }) + + it('keeps ~ verbatim when HOME and USERPROFILE are both unset', () => { + mockPlatform.mockReturnValue('darwin') + vi.stubEnv('PATH', '') + vi.stubEnv('HOME', '') + vi.stubEnv('USERPROFILE', '') + + const result = createEnhancedPath() + const entries = result.split(':') + + expect(entries).toContain('~/.rd/bin') + expect(entries).not.toContain('/.rd/bin') + }) + + it('uses semicolon as separator on win32', () => { + mockPlatform.mockReturnValue('win32') + vi.stubEnv('PATH', 'C:\\Windows\\System32;C:\\Windows') + + const result = createEnhancedPath() + const entries = result.split(';') + + expect(entries[0]).toBe('C:\\Program Files\\Docker\\Docker\\resources\\bin') + expect(entries[1]).toBe('C:\\Program Files\\RedHat\\Podman') + expect(entries).toContain('C:\\Windows\\System32') + expect(entries).toContain('C:\\Windows') + }) + + it('prepends linux container-tooling paths', () => { + mockPlatform.mockReturnValue('linux') + vi.stubEnv('PATH', '/usr/bin:/bin') + vi.stubEnv('HOME', '/home/test') + + const result = createEnhancedPath() + const entries = result.split(':') + + expect(entries.slice(0, 4)).toEqual([ + '/usr/local/bin', + '/opt/homebrew/bin', + '/snap/bin', + '/home/test/.rd/bin', + ]) + }) + + it('preserves existing PATH entries after the prepended paths', () => { + mockPlatform.mockReturnValue('darwin') + vi.stubEnv('PATH', '/existing/bin:/another/bin') + vi.stubEnv('HOME', '/Users/test') + + const result = createEnhancedPath() + const entries = result.split(':') + + const existingIndex = entries.indexOf('/existing/bin') + const anotherIndex = entries.indexOf('/another/bin') + + expect(existingIndex).toBeGreaterThan(-1) + expect(anotherIndex).toBeGreaterThan(existingIndex) + }) + + it('handles empty PATH gracefully', () => { + mockPlatform.mockReturnValue('darwin') + vi.stubEnv('PATH', '') + vi.stubEnv('HOME', '/Users/test') + + const result = createEnhancedPath() + + expect(result).not.toBe('') + expect(result).toContain('/usr/local/bin') + // No trailing empty segments from the split/join + expect(result.endsWith(':')).toBe(false) + }) +}) diff --git a/main/src/utils/enhanced-path.ts b/main/src/utils/enhanced-path.ts new file mode 100644 index 000000000..6ce4f0fba --- /dev/null +++ b/main/src/utils/enhanced-path.ts @@ -0,0 +1,40 @@ +import { platform } from 'node:os' + +const getCommonPaths = (): string[] => { + const currentPlatform = platform() + + switch (currentPlatform) { + case 'darwin': + return [ + '/Applications/Docker.app/Contents/Resources/bin', + '/opt/homebrew/bin', + '/usr/local/bin', + '~/.rd/bin', + ] + case 'linux': + return ['/usr/local/bin', '/opt/homebrew/bin', '/snap/bin', '~/.rd/bin'] + case 'win32': + return [ + 'C:\\Program Files\\Docker\\Docker\\resources\\bin', + 'C:\\Program Files\\RedHat\\Podman', + ] + default: + return [] + } +} + +const expandPath = (path: string): string => { + if (!path.startsWith('~')) return path + const homeDir = process.env.HOME || process.env.USERPROFILE + return homeDir ? path.replace('~', homeDir) : path +} + +export const createEnhancedPath = (): string => { + const commonPaths = getCommonPaths().map(expandPath) + const currentPath = process.env.PATH || '' + const separator = platform() === 'win32' ? ';' : ':' + + return [...commonPaths, ...currentPath.split(separator)] + .filter(Boolean) + .join(separator) +}