From 44a0454a49225daa48aa9e7fb8eb351ef2d1953a Mon Sep 17 00:00:00 2001 From: Samuele Verzi Date: Fri, 24 Apr 2026 10:52:53 +0200 Subject: [PATCH 1/3] refactor(main): extract createEnhancedPath helper into shared util --- main/src/container-engine.ts | 40 +----- .../src/utils/__tests__/enhanced-path.test.ts | 115 ++++++++++++++++++ main/src/utils/enhanced-path.ts | 39 ++++++ 3 files changed, 155 insertions(+), 39 deletions(-) create mode 100644 main/src/utils/__tests__/enhanced-path.test.ts create mode 100644 main/src/utils/enhanced-path.ts 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/utils/__tests__/enhanced-path.test.ts b/main/src/utils/__tests__/enhanced-path.test.ts new file mode 100644 index 000000000..db8ceb992 --- /dev/null +++ b/main/src/utils/__tests__/enhanced-path.test.ts @@ -0,0 +1,115 @@ +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('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..113d95853 --- /dev/null +++ b/main/src/utils/enhanced-path.ts @@ -0,0 +1,39 @@ +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 => + path.startsWith('~') + ? path.replace('~', process.env.HOME || process.env.USERPROFILE || '') + : 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) +} From d9156b632be8386a1f40550a899fca28cffbd4e0 Mon Sep 17 00:00:00 2001 From: Samuele Verzi Date: Fri, 24 Apr 2026 10:52:58 +0200 Subject: [PATCH 2/3] fix(main): enhance PATH when spawning thv so macOS credential helpers resolve --- main/src/tests/toolhive-manager.test.ts | 43 +++++++++++++++++++++++++ main/src/toolhive-manager.ts | 2 ++ 2 files changed, 45 insertions(+) diff --git a/main/src/tests/toolhive-manager.test.ts b/main/src/tests/toolhive-manager.test.ts index e56ba50a3..0a49e111c 100644 --- a/main/src/tests/toolhive-manager.test.ts +++ b/main/src/tests/toolhive-manager.test.ts @@ -454,6 +454,49 @@ 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 () => { + const originalPath = process.env.PATH + const originalPlatform = process.platform + Object.defineProperty(process, 'platform', { + value: 'darwin', + configurable: true, + }) + vi.stubEnv('PATH', '/usr/bin:/bin') + + try { + 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') + } finally { + Object.defineProperty(process, 'platform', { + value: originalPlatform, + configurable: true, + }) + if (originalPath === undefined) { + delete process.env.PATH + } else { + process.env.PATH = originalPath + } + } + }) + 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', }, }) From 1a6733d52c8d41e35286e420e64ab245f5f3cd7d Mon Sep 17 00:00:00 2001 From: Samuele Verzi Date: Fri, 24 Apr 2026 11:10:50 +0200 Subject: [PATCH 3/3] refactor(main): address PR feedback on enhanced-path helper --- main/src/tests/toolhive-manager.test.ts | 68 +++++++++---------- .../src/utils/__tests__/enhanced-path.test.ts | 13 ++++ main/src/utils/enhanced-path.ts | 9 +-- 3 files changed, 50 insertions(+), 40 deletions(-) diff --git a/main/src/tests/toolhive-manager.test.ts b/main/src/tests/toolhive-manager.test.ts index 0a49e111c..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() @@ -455,46 +468,29 @@ describe('toolhive-manager', () => { }) it('spawns with an enhanced PATH that includes common container-tooling dirs', async () => { - const originalPath = process.env.PATH - const originalPlatform = process.platform - Object.defineProperty(process, 'platform', { - value: 'darwin', - configurable: true, - }) + vi.mocked(platform).mockReturnValue('darwin') vi.stubEnv('PATH', '/usr/bin:/bin') - try { - 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(':') + const startPromise = startToolhive() + await vi.advanceTimersByTimeAsync(50) + await startPromise - 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') - } finally { - Object.defineProperty(process, 'platform', { - value: originalPlatform, - configurable: true, - }) - if (originalPath === undefined) { - delete process.env.PATH - } else { - process.env.PATH = originalPath - } + 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 () => { diff --git a/main/src/utils/__tests__/enhanced-path.test.ts b/main/src/utils/__tests__/enhanced-path.test.ts index db8ceb992..3252adc74 100644 --- a/main/src/utils/__tests__/enhanced-path.test.ts +++ b/main/src/utils/__tests__/enhanced-path.test.ts @@ -56,6 +56,19 @@ describe('createEnhancedPath', () => { 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') diff --git a/main/src/utils/enhanced-path.ts b/main/src/utils/enhanced-path.ts index 113d95853..6ce4f0fba 100644 --- a/main/src/utils/enhanced-path.ts +++ b/main/src/utils/enhanced-path.ts @@ -23,10 +23,11 @@ const getCommonPaths = (): string[] => { } } -const expandPath = (path: string): string => - path.startsWith('~') - ? path.replace('~', process.env.HOME || process.env.USERPROFILE || '') - : path +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)