diff --git a/src/__tests__/cli/services/agent-spawner.test.ts b/src/__tests__/cli/services/agent-spawner.test.ts index d888907dc9..aecdc1ff59 100644 --- a/src/__tests__/cli/services/agent-spawner.test.ts +++ b/src/__tests__/cli/services/agent-spawner.test.ts @@ -46,13 +46,24 @@ vi.mock('child_process', async (importOriginal) => { }); // Mock fs module -vi.mock('fs', async (importOriginal) => { - const actual = await importOriginal(); - return { +vi.mock('fs', async () => { + const actual = await vi.importActual('fs'); + const mocked = { ...actual, readFileSync: vi.fn(), writeFileSync: vi.fn(), + existsSync: vi.fn(() => false), + readdirSync: vi.fn(() => []), + mkdirSync: vi.fn(), + createWriteStream: vi.fn( + () => + ({ + write: vi.fn(), + end: vi.fn(), + }) as any + ), promises: { + ...actual.promises, stat: vi.fn(), access: vi.fn(), }, @@ -60,12 +71,25 @@ vi.mock('fs', async (importOriginal) => { X_OK: 1, }, }; + return { + ...mocked, + default: mocked, + }; }); // Mock os module -vi.mock('os', () => ({ - homedir: vi.fn(() => '/Users/testuser'), -})); +vi.mock('os', async () => { + const actual = await vi.importActual('os'); + const mocked = { + ...actual, + homedir: vi.fn(() => '/Users/testuser'), + tmpdir: vi.fn(() => '/tmp'), + }; + return { + ...mocked, + default: mocked, + }; +}); // Mock storage service const mockGetAgentCustomPath = vi.fn(); diff --git a/src/__tests__/main/agents/session-storage.test.ts b/src/__tests__/main/agents/session-storage.test.ts index 28ff3f88b3..f1abcae301 100644 --- a/src/__tests__/main/agents/session-storage.test.ts +++ b/src/__tests__/main/agents/session-storage.test.ts @@ -22,6 +22,7 @@ vi.mock('os', async () => { const mocked = { ...actual, homedir: vi.fn(() => '/tmp/maestro-session-storage-home'), + tmpdir: vi.fn(() => '/tmp'), }; return { ...mocked, diff --git a/src/__tests__/shared/pathUtils.test.ts b/src/__tests__/shared/pathUtils.test.ts index 39c2c9acf7..ce7c0d8b1a 100644 --- a/src/__tests__/shared/pathUtils.test.ts +++ b/src/__tests__/shared/pathUtils.test.ts @@ -14,6 +14,8 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import * as os from 'os'; +import * as fs from 'fs'; +import * as path from 'path'; import { expandTilde, parseVersion, @@ -28,6 +30,7 @@ vi.mock('os', async () => { return { ...actual, homedir: vi.fn(() => '/Users/testuser'), + tmpdir: () => '/tmp', }; }); @@ -276,6 +279,35 @@ describe('buildExpandedPath', () => { expect(homebrewIndex).toBeLessThan(customIndex); }); + it('should prepend detected Node version manager bin paths', () => { + process.env.PATH = '/usr/bin'; + const originalNvmDir = process.env.NVM_DIR; + const tempNvmDir = fs.mkdtempSync(path.join(os.tmpdir(), 'maestro-nvm-')); + process.env.NVM_DIR = tempNvmDir; + fs.mkdirSync(path.join(tempNvmDir, 'current', 'bin'), { recursive: true }); + fs.mkdirSync(path.join(tempNvmDir, 'versions', 'node', 'v22.10.0', 'bin'), { + recursive: true, + }); + + try { + const result = buildExpandedPath(); + const pathParts = result.split(':'); + const currentBin = path.join(tempNvmDir, 'current', 'bin'); + const versionedBin = path.join(tempNvmDir, 'versions', 'node', 'v22.10.0', 'bin'); + + expect(pathParts[0]).toBe(currentBin); + expect(pathParts).toContain(versionedBin); + expect(pathParts.indexOf(currentBin)).toBeLessThan(pathParts.indexOf(versionedBin)); + } finally { + if (originalNvmDir === undefined) { + delete process.env.NVM_DIR; + } else { + process.env.NVM_DIR = originalNvmDir; + } + fs.rmSync(tempNvmDir, { recursive: true, force: true }); + } + }); + it('should accept custom paths that are prepended first', () => { process.env.PATH = '/usr/bin'; const result = buildExpandedPath(['/my/custom/bin', '/another/path']); diff --git a/src/main/process-manager/utils/__tests__/envBuilder.test.ts b/src/main/process-manager/utils/__tests__/envBuilder.test.ts index 68443c0dd4..2b3e60b4f6 100644 --- a/src/main/process-manager/utils/__tests__/envBuilder.test.ts +++ b/src/main/process-manager/utils/__tests__/envBuilder.test.ts @@ -11,6 +11,7 @@ */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import { buildChildProcessEnv, buildPtyTerminalEnv } from '../envBuilder'; @@ -304,6 +305,37 @@ describe('envBuilder - Global Environment Variables', () => { // The actual value depends on the system, but it should exist expect((env.PATH as string).length).toBeGreaterThan(0); }); + + it('should include detected Node version manager bins in PATH', () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'darwin' }); + const originalNvmDir = process.env.NVM_DIR; + const tempNvmDir = fs.mkdtempSync(path.join(os.tmpdir(), 'maestro-nvm-')); + process.env.NVM_DIR = tempNvmDir; + fs.mkdirSync(path.join(tempNvmDir, 'current', 'bin'), { recursive: true }); + fs.mkdirSync(path.join(tempNvmDir, 'versions', 'node', 'v22.10.0', 'bin'), { + recursive: true, + }); + + try { + const env = buildChildProcessEnv(); + const pathParts = env.PATH?.split(path.delimiter) || []; + const currentBin = path.join(tempNvmDir, 'current', 'bin'); + const versionedBin = path.join(tempNvmDir, 'versions', 'node', 'v22.10.0', 'bin'); + + expect(pathParts[0]).toBe(currentBin); + expect(pathParts).toContain(versionedBin); + expect(pathParts.indexOf(currentBin)).toBeLessThan(pathParts.indexOf(versionedBin)); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform }); + if (originalNvmDir === undefined) { + delete process.env.NVM_DIR; + } else { + process.env.NVM_DIR = originalNvmDir; + } + fs.rmSync(tempNvmDir, { recursive: true, force: true }); + } + }); }); describe('Test 2.6: Complex Precedence Chain', () => { diff --git a/src/shared/pathUtils.ts b/src/shared/pathUtils.ts index 49e0a2f4f5..391434053b 100644 --- a/src/shared/pathUtils.ts +++ b/src/shared/pathUtils.ts @@ -302,6 +302,7 @@ export function detectNodeVersionManagerBinPaths(): string[] { export function buildExpandedPath(customPaths?: string[]): string { const delimiter = path.delimiter; const home = os.homedir(); + const versionManagerPaths = detectNodeVersionManagerBinPaths(); // Start with current PATH const currentPath = process.env.PATH || ''; @@ -370,6 +371,7 @@ export function buildExpandedPath(customPaths?: string[]): string { } else { // Unix-like paths (macOS/Linux) additionalPaths = [ + ...versionManagerPaths, '/opt/homebrew/bin', // Homebrew on Apple Silicon '/opt/homebrew/sbin', '/usr/local/bin', // Homebrew on Intel, common install location @@ -386,9 +388,11 @@ export function buildExpandedPath(customPaths?: string[]): string { ]; } - // Add custom paths first (if provided) + // Iterate in reverse because each entry is prepended with unshift(). + // This preserves the caller's intended left-to-right path precedence. if (customPaths && customPaths.length > 0) { - for (const p of customPaths) { + for (let i = customPaths.length - 1; i >= 0; i--) { + const p = customPaths[i]; if (!pathParts.includes(p)) { pathParts.unshift(p); } @@ -396,7 +400,8 @@ export function buildExpandedPath(customPaths?: string[]): string { } // Add standard additional paths - for (const p of additionalPaths) { + for (let i = additionalPaths.length - 1; i >= 0; i--) { + const p = additionalPaths[i]; if (!pathParts.includes(p)) { pathParts.unshift(p); }