From f2ba4731efcae7650c5c2a652f4c6009ff1c3f85 Mon Sep 17 00:00:00 2001 From: Mike Brown II Date: Thu, 12 Mar 2026 14:55:37 -0400 Subject: [PATCH 1/4] Include version manager bins in expanded PATH Packaged Maestro can detect Node-backed CLIs like Codex in nvm or fnm installations but still fail to launch them. Agent detection probes those locations directly, while child-process PATH expansion previously omitted the detected version manager bin directories. Add detected version manager bins to the shared expanded PATH and preserve the intended precedence when prepending paths. This keeps nvm/current/bin ahead of versioned installs and prevents packaged launches from failing with 'env: node: No such file or directory'. Add regression coverage for both the shared PATH builder and the child process environment builder so packaged-path resolution stays aligned with agent detection. --- src/__tests__/shared/pathUtils.test.ts | 31 +++++++++++++++++++ .../utils/__tests__/envBuilder.test.ts | 29 +++++++++++++++++ src/shared/pathUtils.ts | 8 +++-- 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/src/__tests__/shared/pathUtils.test.ts b/src/__tests__/shared/pathUtils.test.ts index 39c2c9acf7..3ecd14c33c 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, @@ -276,6 +278,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('/tmp/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..d8f1d93ef6 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,34 @@ 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 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 { + 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..f9f8d0a118 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 @@ -388,7 +390,8 @@ export function buildExpandedPath(customPaths?: string[]): string { // Add custom paths first (if provided) 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 +399,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); } From a62875fca23deeabe87d339f18791c0dec13d9d9 Mon Sep 17 00:00:00 2001 From: Mike Brown II Date: Thu, 12 Mar 2026 15:32:01 -0400 Subject: [PATCH 2/4] Address PR review feedback Use os.tmpdir() in the pathUtils regression test instead of a hardcoded temporary directory path, and clarify the PATH-prepend comment to explain why the arrays are iterated in reverse. These changes keep the implementation the same while addressing the review notes on portability and code clarity. --- src/__tests__/shared/pathUtils.test.ts | 3 ++- src/shared/pathUtils.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/__tests__/shared/pathUtils.test.ts b/src/__tests__/shared/pathUtils.test.ts index 3ecd14c33c..ce7c0d8b1a 100644 --- a/src/__tests__/shared/pathUtils.test.ts +++ b/src/__tests__/shared/pathUtils.test.ts @@ -30,6 +30,7 @@ vi.mock('os', async () => { return { ...actual, homedir: vi.fn(() => '/Users/testuser'), + tmpdir: () => '/tmp', }; }); @@ -281,7 +282,7 @@ describe('buildExpandedPath', () => { it('should prepend detected Node version manager bin paths', () => { process.env.PATH = '/usr/bin'; const originalNvmDir = process.env.NVM_DIR; - const tempNvmDir = fs.mkdtempSync('/tmp/maestro-nvm-'); + 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'), { diff --git a/src/shared/pathUtils.ts b/src/shared/pathUtils.ts index f9f8d0a118..391434053b 100644 --- a/src/shared/pathUtils.ts +++ b/src/shared/pathUtils.ts @@ -388,7 +388,8 @@ 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 (let i = customPaths.length - 1; i >= 0; i--) { const p = customPaths[i]; From 8bf9f7d39bb5ef0bfa63f1e0b193c4d8cb7bb4d3 Mon Sep 17 00:00:00 2001 From: Mike Brown II Date: Thu, 12 Mar 2026 15:42:49 -0400 Subject: [PATCH 3/4] Make envBuilder PATH test platform-safe Force the regression test to run under a Unix-like platform value so it exercises version manager path detection consistently across local runs and CI, including Windows. This addresses the remaining review concern without changing the production code path. --- src/main/process-manager/utils/__tests__/envBuilder.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/process-manager/utils/__tests__/envBuilder.test.ts b/src/main/process-manager/utils/__tests__/envBuilder.test.ts index d8f1d93ef6..2b3e60b4f6 100644 --- a/src/main/process-manager/utils/__tests__/envBuilder.test.ts +++ b/src/main/process-manager/utils/__tests__/envBuilder.test.ts @@ -307,6 +307,8 @@ describe('envBuilder - Global Environment Variables', () => { }); 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; @@ -325,6 +327,7 @@ describe('envBuilder - Global Environment Variables', () => { 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 { From 2f5be1469f401461128cbdc4ba0ed1f69f5fac18 Mon Sep 17 00:00:00 2001 From: Mike Brown II <2635749+brownmike@users.noreply.github.com> Date: Thu, 12 Mar 2026 18:36:18 -0400 Subject: [PATCH 4/4] Fix CI fallout from PATH expansion tests --- .../cli/services/agent-spawner.test.ts | 36 +++++++++++++++---- .../main/agents/session-storage.test.ts | 1 + 2 files changed, 31 insertions(+), 6 deletions(-) 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,