From 13b14762bc92c2d0c962df6025b9b0a23da4b900 Mon Sep 17 00:00:00 2001 From: 777genius Date: Fri, 22 May 2026 00:17:54 +0300 Subject: [PATCH] fix(runtime): bound binary path discovery --- .../composition/createCodexAccountFeature.ts | 1 + .../CodexRuntimeInstallerService.ts | 61 +++--------- .../output/sources/TmuxStatusSourceAdapter.ts | 2 - .../installer/TmuxInstallStrategyResolver.ts | 3 +- .../runtime/TmuxPlatformCommandExecutor.ts | 6 +- .../OpenCodeRuntimeInstallerService.ts | 81 +++------------ .../codexAppServer/CodexBinaryResolver.ts | 2 + .../schedule/ScheduledTaskExecutor.ts | 2 +- .../services/team/ClaudeBinaryResolver.ts | 10 +- src/main/utils/runtimePathBinaryResolver.ts | 92 +++++++++++++++++ src/main/utils/shellEnv.ts | 43 +++++++- .../CodexRuntimeInstallerService.test.ts | 2 +- .../team/ClaudeBinaryResolver.test.ts | 25 +++-- .../utils/runtimePathBinaryResolver.test.ts | 99 +++++++++++++++++++ test/main/utils/shellEnv.integration.test.ts | 46 +++++++-- test/main/utils/shellEnv.test.ts | 66 ++++++++++++- 16 files changed, 390 insertions(+), 151 deletions(-) create mode 100644 src/main/utils/runtimePathBinaryResolver.ts create mode 100644 test/main/utils/runtimePathBinaryResolver.test.ts diff --git a/src/features/codex-account/main/composition/createCodexAccountFeature.ts b/src/features/codex-account/main/composition/createCodexAccountFeature.ts index 4810bf96..5a9f39ec 100644 --- a/src/features/codex-account/main/composition/createCodexAccountFeature.ts +++ b/src/features/codex-account/main/composition/createCodexAccountFeature.ts @@ -263,6 +263,7 @@ async function resolveCodexBinaryForAccountSnapshot(): Promise { await resolveInteractiveShellEnvBestEffort({ timeoutMs: CODEX_BINARY_COLD_RETRY_TIMEOUT_MS, fallbackEnv: process.env, + background: false, }); CodexBinaryResolver.clearCache(); return CodexBinaryResolver.resolve(); diff --git a/src/features/codex-runtime-installer/main/infrastructure/CodexRuntimeInstallerService.ts b/src/features/codex-runtime-installer/main/infrastructure/CodexRuntimeInstallerService.ts index 2016acab..7490f215 100644 --- a/src/features/codex-runtime-installer/main/infrastructure/CodexRuntimeInstallerService.ts +++ b/src/features/codex-runtime-installer/main/infrastructure/CodexRuntimeInstallerService.ts @@ -1,13 +1,17 @@ import { CODEX_RUNTIME_PROGRESS } from '@features/codex-runtime-installer/contracts'; import { execCli } from '@main/utils/childProcess'; -import { buildMergedCliPath } from '@main/utils/cliPathMerge'; import { getAppDataPath } from '@main/utils/pathDecoder'; +import { + findFirstRuntimePathBinaryCandidate, + isAbsoluteExistingFile, + RUNTIME_PATH_SHELL_ENV_TIMEOUT_MS, +} from '@main/utils/runtimePathBinaryResolver'; import { safeSendToRenderer } from '@main/utils/safeWebContentsSend'; -import { getCachedShellEnv, resolveInteractiveShellEnvBestEffort } from '@main/utils/shellEnv'; +import { resolveInteractiveShellEnvBestEffort } from '@main/utils/shellEnv'; import { getErrorMessage } from '@shared/utils/errorHandling'; import { createLogger } from '@shared/utils/logger'; import { createHash, randomUUID } from 'crypto'; -import { existsSync, promises as fsp, readFileSync, statSync } from 'fs'; +import { promises as fsp, readFileSync } from 'fs'; import path from 'path'; import { gunzipSync } from 'zlib'; @@ -28,7 +32,6 @@ const MAX_TARBALL_BYTES = 160 * 1024 * 1024; const MAX_UNPACKED_BYTES = 650 * 1024 * 1024; const FETCH_TIMEOUT_MS = 60_000; const VERSION_TIMEOUT_MS = 10_000; -const PATH_SHELL_ENV_TIMEOUT_MS = 1_500; interface NpmPackageMetadata { name?: string; @@ -71,17 +74,6 @@ function getCurrentManifestPath(): string { return path.join(getRuntimeRootPath(), 'current.json'); } -function isAbsoluteExistingFile(filePath: string | null | undefined): filePath is string { - if (!filePath || !path.isAbsolute(filePath) || !existsSync(filePath)) { - return false; - } - try { - return statSync(filePath).isFile(); - } catch { - return false; - } -} - function parseManifest(value: unknown): CodexRuntimeManifest | null { if (!value || typeof value !== 'object' || Array.isArray(value)) { return null; @@ -141,41 +133,13 @@ function getPathExecutableNames(): string[] { : ['codex']; } -function splitPathEnv(pathValue: string | undefined): string[] { - if (!pathValue) { - return []; - } - return pathValue - .split(path.delimiter) - .map((entry) => entry.trim()) - .filter(Boolean); -} - function resolvePathCodexBinary( additionalEnvSources: (NodeJS.ProcessEnv | null | undefined)[] = [] ): string | null { - const shellEnv = getCachedShellEnv() ?? {}; - const pathEntries = [ - ...additionalEnvSources.flatMap((env) => splitPathEnv(env?.PATH)), - ...splitPathEnv(shellEnv.PATH), - ...splitPathEnv(buildMergedCliPath(null)), - ...splitPathEnv(process.env.PATH), - ]; - const seen = new Set(); - for (const entry of pathEntries) { - const normalizedEntry = path.resolve(entry); - if (seen.has(normalizedEntry)) { - continue; - } - seen.add(normalizedEntry); - for (const executableName of getPathExecutableNames()) { - const candidate = path.join(normalizedEntry, executableName); - if (isAbsoluteExistingFile(candidate)) { - return candidate; - } - } - } - return null; + return findFirstRuntimePathBinaryCandidate({ + executableNames: getPathExecutableNames(), + additionalEnvSources, + }); } async function resolvePathCodexBinaryWithBestEffortEnv( @@ -187,8 +151,9 @@ async function resolvePathCodexBinaryWithBestEffortEnv( } const shellEnv = await resolveInteractiveShellEnvBestEffort({ - timeoutMs: options.shellEnvTimeoutMs ?? PATH_SHELL_ENV_TIMEOUT_MS, + timeoutMs: options.shellEnvTimeoutMs ?? RUNTIME_PATH_SHELL_ENV_TIMEOUT_MS, fallbackEnv: process.env, + background: false, }); return resolvePathCodexBinary([shellEnv]); } diff --git a/src/features/tmux-installer/main/adapters/output/sources/TmuxStatusSourceAdapter.ts b/src/features/tmux-installer/main/adapters/output/sources/TmuxStatusSourceAdapter.ts index fd2ae6eb..74cbc0f8 100644 --- a/src/features/tmux-installer/main/adapters/output/sources/TmuxStatusSourceAdapter.ts +++ b/src/features/tmux-installer/main/adapters/output/sources/TmuxStatusSourceAdapter.ts @@ -6,7 +6,6 @@ import { TmuxPackageManagerResolver } from '@features/tmux-installer/main/infras import { TmuxPlatformResolver } from '@features/tmux-installer/main/infrastructure/platform/TmuxPlatformResolver'; import { TmuxWslService } from '@features/tmux-installer/main/infrastructure/wsl/TmuxWslService'; import { buildEnrichedEnv } from '@main/utils/cliEnv'; -import { resolveInteractiveShellEnv } from '@main/utils/shellEnv'; import { getErrorMessage } from '@shared/utils/errorHandling'; import type { @@ -83,7 +82,6 @@ export class TmuxStatusSourceAdapter implements TmuxStatusSourcePort { async #probeStatus(): Promise { const resolvedPlatform = await this.#platformResolver.resolve(); const checkedAt = new Date().toISOString(); - await resolveInteractiveShellEnv(); const env = buildEnrichedEnv(); const plan = await this.#strategyResolver.resolve(); diff --git a/src/features/tmux-installer/main/infrastructure/installer/TmuxInstallStrategyResolver.ts b/src/features/tmux-installer/main/infrastructure/installer/TmuxInstallStrategyResolver.ts index c7864f80..d64e7141 100644 --- a/src/features/tmux-installer/main/infrastructure/installer/TmuxInstallStrategyResolver.ts +++ b/src/features/tmux-installer/main/infrastructure/installer/TmuxInstallStrategyResolver.ts @@ -1,6 +1,6 @@ import { buildTmuxAutoInstallCapability } from '@features/tmux-installer/core/domain/policies/buildTmuxAutoInstallCapability'; import { buildEnrichedEnv } from '@main/utils/cliEnv'; -import { getShellPreferredHome, resolveInteractiveShellEnv } from '@main/utils/shellEnv'; +import { getShellPreferredHome } from '@main/utils/shellEnv'; import { TmuxPackageManagerResolver } from '../platform/TmuxPackageManagerResolver'; import { TmuxPlatformResolver } from '../platform/TmuxPlatformResolver'; @@ -51,7 +51,6 @@ export class TmuxInstallStrategyResolver { } async resolve(): Promise { - await resolveInteractiveShellEnv(); const env = buildEnrichedEnv(); const cwd = getShellPreferredHome(); const resolvedPlatform = await this.#platformResolver.resolve(); diff --git a/src/features/tmux-installer/main/infrastructure/runtime/TmuxPlatformCommandExecutor.ts b/src/features/tmux-installer/main/infrastructure/runtime/TmuxPlatformCommandExecutor.ts index d5e434a7..a3409800 100644 --- a/src/features/tmux-installer/main/infrastructure/runtime/TmuxPlatformCommandExecutor.ts +++ b/src/features/tmux-installer/main/infrastructure/runtime/TmuxPlatformCommandExecutor.ts @@ -4,7 +4,6 @@ import * as os from 'node:os'; import * as path from 'node:path'; import { buildEnrichedEnv } from '@main/utils/cliEnv'; -import { resolveInteractiveShellEnv } from '@main/utils/shellEnv'; import { TmuxPackageManagerResolver } from '../platform/TmuxPackageManagerResolver'; import { TmuxWslService } from '../wsl/TmuxWslService'; @@ -71,7 +70,6 @@ export class TmuxPlatformCommandExecutor { return this.#wslService.execTmux(effectiveArgs, null, timeout); } - await resolveInteractiveShellEnv(); const env = buildEnrichedEnv(); const executable = await this.#resolveNativeTmuxExecutable(env); return new Promise((resolve) => { @@ -250,13 +248,11 @@ export class TmuxPlatformCommandExecutor { } async #execNativePs(): Promise { - await resolveInteractiveShellEnv(); - const env = buildEnrichedEnv(); return new Promise((resolve) => { execFile( 'ps', ['-ax', '-o', 'pid=,ppid=,command='], - { env, timeout: 3_000, maxBuffer: 2 * 1024 * 1024 }, + { env: process.env, timeout: 3_000, maxBuffer: 2 * 1024 * 1024 }, (error, stdout, stderr) => { const errorCode = typeof error === 'object' && error !== null && 'code' in error diff --git a/src/main/services/infrastructure/OpenCodeRuntimeInstallerService.ts b/src/main/services/infrastructure/OpenCodeRuntimeInstallerService.ts index 3a546211..dada93fb 100644 --- a/src/main/services/infrastructure/OpenCodeRuntimeInstallerService.ts +++ b/src/main/services/infrastructure/OpenCodeRuntimeInstallerService.ts @@ -1,16 +1,16 @@ import { execCli } from '@main/utils/childProcess'; -import { buildMergedCliPath } from '@main/utils/cliPathMerge'; import { getAppDataPath } from '@main/utils/pathDecoder'; -import { safeSendToRenderer } from '@main/utils/safeWebContentsSend'; import { - getCachedShellEnv, - getShellPreferredHome, - resolveInteractiveShellEnvBestEffort, -} from '@main/utils/shellEnv'; + collectRuntimePathBinaryCandidates, + isAbsoluteExistingFile, + RUNTIME_PATH_SHELL_ENV_TIMEOUT_MS, +} from '@main/utils/runtimePathBinaryResolver'; +import { safeSendToRenderer } from '@main/utils/safeWebContentsSend'; +import { getShellPreferredHome, resolveInteractiveShellEnvBestEffort } from '@main/utils/shellEnv'; import { getErrorMessage } from '@shared/utils/errorHandling'; import { createLogger } from '@shared/utils/logger'; import { createHash, randomUUID } from 'crypto'; -import { existsSync, promises as fsp, readdirSync, readFileSync, statSync } from 'fs'; +import { promises as fsp, readdirSync, readFileSync } from 'fs'; import path from 'path'; import { gunzipSync } from 'zlib'; @@ -27,7 +27,6 @@ const MAX_TARBALL_BYTES = 250 * 1024 * 1024; const MAX_BINARY_BYTES = 350 * 1024 * 1024; const FETCH_TIMEOUT_MS = 60_000; const VERSION_TIMEOUT_MS = 10_000; -const PATH_SHELL_ENV_TIMEOUT_MS = 1_500; interface NpmPackageMetadata { name?: string; @@ -61,17 +60,6 @@ function getCurrentManifestPath(): string { return path.join(getRuntimeRootPath(), 'current.json'); } -function isAbsoluteExistingFile(filePath: string | null | undefined): filePath is string { - if (!filePath || !path.isAbsolute(filePath) || !existsSync(filePath)) { - return false; - } - try { - return statSync(filePath).isFile(); - } catch { - return false; - } -} - function parseManifest(value: unknown): OpenCodeRuntimeManifest | null { if (!value || typeof value !== 'object' || Array.isArray(value)) { return null; @@ -130,56 +118,16 @@ function getPathExecutableNames(): string[] { : ['opencode']; } -function splitPathEnv(pathValue: string | undefined): string[] { - if (!pathValue) { - return []; - } - return pathValue - .split(path.delimiter) - .map((entry) => entry.trim()) - .filter(Boolean); -} - function collectPathOpenCodeBinaryCandidates( additionalEnvSources: (NodeJS.ProcessEnv | null | undefined)[] = [], options: { includeFallbackPathEntries?: boolean } = {} ): string[] { - const shellEnv = getCachedShellEnv() ?? {}; - const directPathEntries = [ - ...additionalEnvSources.flatMap((env) => splitPathEnv(env?.PATH)), - ...splitPathEnv(shellEnv.PATH), - ]; - const fallbackPathEntries = - options.includeFallbackPathEntries === false - ? [] - : [...splitPathEnv(buildMergedCliPath()), ...splitPathEnv(process.env.PATH)]; - const seen = new Set(); - return [ - ...collectOpenCodeBinariesFromPathEntries(directPathEntries, seen), - ...collectNvmOpenCodeBinaryCandidates().filter(isAbsoluteExistingFile), - ...collectOpenCodeBinariesFromPathEntries(fallbackPathEntries, seen), - ]; -} - -function collectOpenCodeBinariesFromPathEntries( - pathEntries: string[], - seen: Set -): string[] { - const results: string[] = []; - for (const entry of pathEntries) { - const normalizedEntry = path.resolve(entry); - if (seen.has(normalizedEntry)) { - continue; - } - seen.add(normalizedEntry); - for (const executableName of getPathExecutableNames()) { - const candidate = path.join(normalizedEntry, executableName); - if (isAbsoluteExistingFile(candidate)) { - results.push(candidate); - } - } - } - return results; + return collectRuntimePathBinaryCandidates({ + executableNames: getPathExecutableNames(), + additionalEnvSources, + includeFallbackPathEntries: options.includeFallbackPathEntries, + extraCandidates: collectNvmOpenCodeBinaryCandidates(), + }); } function collectNvmOpenCodeBinaryCandidates(): string[] { @@ -283,8 +231,9 @@ async function probeFirstWorkingPathOpenCodeBinary( firstFailure = cachedProbe.firstFailure; const shellEnv = await resolveInteractiveShellEnvBestEffort({ - timeoutMs: options.shellEnvTimeoutMs ?? PATH_SHELL_ENV_TIMEOUT_MS, + timeoutMs: options.shellEnvTimeoutMs ?? RUNTIME_PATH_SHELL_ENV_TIMEOUT_MS, fallbackEnv: process.env, + background: false, }); const shellProbe = await probeFirstWorkingOpenCodeBinaryCandidate( collectPathOpenCodeBinaryCandidates([shellEnv], { diff --git a/src/main/services/infrastructure/codexAppServer/CodexBinaryResolver.ts b/src/main/services/infrastructure/codexAppServer/CodexBinaryResolver.ts index da73944c..f14d253b 100644 --- a/src/main/services/infrastructure/codexAppServer/CodexBinaryResolver.ts +++ b/src/main/services/infrastructure/codexAppServer/CodexBinaryResolver.ts @@ -73,6 +73,8 @@ function isPathLikeCandidate(candidate: string): boolean { } function getPathEntries(): string[] { + // TODO: Consider sharing runtimePathBinaryResolver here after preserving this resolver's + // path-like candidate support and Windows PATHEXT normalization exactly. const delimiter = process.platform === 'win32' ? ';' : path.delimiter; const shellEnv = getCachedShellEnv() ?? {}; const seen = new Set(); diff --git a/src/main/services/schedule/ScheduledTaskExecutor.ts b/src/main/services/schedule/ScheduledTaskExecutor.ts index 849250d5..01577b0d 100644 --- a/src/main/services/schedule/ScheduledTaskExecutor.ts +++ b/src/main/services/schedule/ScheduledTaskExecutor.ts @@ -144,7 +144,7 @@ export class ScheduledTaskExecutor { throw new Error('Claude CLI binary not found'); } - const shellEnv = await resolveInteractiveShellEnv(); + const shellEnv = await resolveInteractiveShellEnv({ source: 'scheduled-task-executor' }); validateFastModeLaunchConfig(request.config); diff --git a/src/main/services/team/ClaudeBinaryResolver.ts b/src/main/services/team/ClaudeBinaryResolver.ts index faf4b40d..69ee29b5 100644 --- a/src/main/services/team/ClaudeBinaryResolver.ts +++ b/src/main/services/team/ClaudeBinaryResolver.ts @@ -1,6 +1,6 @@ import { buildMergedCliPath } from '@main/utils/cliPathMerge'; import { getClaudeBasePath } from '@main/utils/pathDecoder'; -import { getShellPreferredHome, resolveInteractiveShellEnv } from '@main/utils/shellEnv'; +import { getShellPreferredHome, resolveInteractiveShellEnvBestEffort } from '@main/utils/shellEnv'; import * as fs from 'fs'; import * as path from 'path'; @@ -124,6 +124,9 @@ async function collectNvmWindowsCandidates(): Promise { } async function resolveFromPathEnv(binaryName: string, pathEnv?: string): Promise { + // TODO: Consider migrating this PATH candidate collection to runtimePathBinaryResolver once + // Claude-specific executable checks, Windows PATHEXT handling, and parallel stat behavior + // can be preserved exactly. const rawPath = pathEnv && pathEnv.length > 0 ? pathEnv : process.env.PATH; if (!rawPath) { return null; @@ -299,7 +302,10 @@ export class ClaudeBinaryResolver { } } - await resolveInteractiveShellEnv({ + await resolveInteractiveShellEnvBestEffort({ + timeoutMs: 1_500, + fallbackEnv: process.env, + background: false, onProgress: (progress) => emitProgress(options, progress.phase, progress.message), }); const enrichedPath = buildMergedCliPath(null); diff --git a/src/main/utils/runtimePathBinaryResolver.ts b/src/main/utils/runtimePathBinaryResolver.ts new file mode 100644 index 00000000..45ec4cfb --- /dev/null +++ b/src/main/utils/runtimePathBinaryResolver.ts @@ -0,0 +1,92 @@ +import { existsSync, statSync } from 'fs'; +import path from 'path'; + +import { buildMergedCliPath } from './cliPathMerge'; +import { getCachedShellEnv } from './shellEnv'; + +export const RUNTIME_PATH_SHELL_ENV_TIMEOUT_MS = 1_500; + +export function isAbsoluteExistingFile(filePath: string | null | undefined): filePath is string { + if (!filePath || !path.isAbsolute(filePath) || !existsSync(filePath)) { + return false; + } + try { + return statSync(filePath).isFile(); + } catch { + return false; + } +} + +export function splitPathEnv(pathValue: string | undefined): string[] { + if (!pathValue) { + return []; + } + return pathValue + .split(path.delimiter) + .map((entry) => entry.trim()) + .filter(Boolean); +} + +function collectExecutableCandidatesFromPathEntries( + pathEntries: string[], + executableNames: string[], + seenPathEntries: Set +): string[] { + const results: string[] = []; + for (const entry of pathEntries) { + const normalizedEntry = path.resolve(entry); + if (seenPathEntries.has(normalizedEntry)) { + continue; + } + seenPathEntries.add(normalizedEntry); + for (const executableName of executableNames) { + const candidate = path.join(normalizedEntry, executableName); + if (isAbsoluteExistingFile(candidate)) { + results.push(candidate); + } + } + } + return results; +} + +export interface RuntimePathBinaryCandidateOptions { + executableNames: string[]; + additionalEnvSources?: (NodeJS.ProcessEnv | null | undefined)[]; + includeFallbackPathEntries?: boolean; + extraCandidates?: string[]; +} + +export function collectRuntimePathBinaryCandidates( + options: RuntimePathBinaryCandidateOptions +): string[] { + const additionalEnvSources = options.additionalEnvSources ?? []; + const shellEnv = getCachedShellEnv() ?? {}; + const directPathEntries = [ + ...additionalEnvSources.flatMap((env) => splitPathEnv(env?.PATH)), + ...splitPathEnv(shellEnv.PATH), + ]; + const fallbackPathEntries = + options.includeFallbackPathEntries === false + ? [] + : [...splitPathEnv(buildMergedCliPath(null)), ...splitPathEnv(process.env.PATH)]; + const seenPathEntries = new Set(); + return [ + ...collectExecutableCandidatesFromPathEntries( + directPathEntries, + options.executableNames, + seenPathEntries + ), + ...(options.extraCandidates ?? []).filter(isAbsoluteExistingFile), + ...collectExecutableCandidatesFromPathEntries( + fallbackPathEntries, + options.executableNames, + seenPathEntries + ), + ]; +} + +export function findFirstRuntimePathBinaryCandidate( + options: RuntimePathBinaryCandidateOptions +): string | null { + return collectRuntimePathBinaryCandidates(options)[0] ?? null; +} diff --git a/src/main/utils/shellEnv.ts b/src/main/utils/shellEnv.ts index f3cd0646..a4f19b7d 100644 --- a/src/main/utils/shellEnv.ts +++ b/src/main/utils/shellEnv.ts @@ -27,18 +27,32 @@ let lastShellEnvFailureMessage: string | null = null; export interface ShellEnvResolveProgress { phase: string; message: string; + source?: string; } export interface ShellEnvResolveOptions { onProgress?: (progress: ShellEnvResolveProgress) => void; + /** + * Stable diagnostic label for the caller that initiated the shell probe. + * Keep this to a short feature/service id, not a filesystem path. + */ + source?: string; } export interface ShellEnvBestEffortResolveOptions extends ShellEnvResolveOptions { /** * Max time to wait on the critical path before returning fallbackEnv. - * The full shell resolve continues in the background and caches on success. + * By default, the full shell resolve continues in the background and caches + * on success. Set background=false for hot paths that only want cached env + * or an immediate fallback. */ timeoutMs?: number; + /** + * Whether a slow shell probe should continue in the background after the + * caller falls back. Disable this for startup/status hot paths where a + * delayed hard timeout would only create log noise and process pressure. + */ + background?: boolean; /** * Returned when shell env is not ready quickly enough. This is intentionally * not cached as a real shell env. @@ -51,7 +65,21 @@ function emitProgress( phase: string, message: string ): void { - options?.onProgress?.({ phase, message }); + const source = normalizeShellEnvSource(options?.source); + options?.onProgress?.(source ? { phase, message, source } : { phase, message }); +} + +function normalizeShellEnvSource(source: string | undefined): string | null { + const trimmed = source?.trim(); + if (!trimmed) { + return null; + } + return trimmed.replace(/[^A-Za-z0-9_.:-]/g, '_').slice(0, 80); +} + +function formatShellEnvSource(options: ShellEnvResolveOptions | undefined): string { + const source = normalizeShellEnvSource(options?.source); + return source ? ` source=${source}` : ''; } function rememberShellEnvFailure(message: string): void { @@ -177,7 +205,6 @@ export async function resolveInteractiveShellEnv( return loginEnv; } catch (loginError) { const loginMessage = loginError instanceof Error ? loginError.message : String(loginError); - logger.warn(`Failed to resolve login shell env: ${loginMessage}`); try { emitProgress(options, 'shell-env-interactive', 'Trying interactive shell environment...'); const interactiveEnv = await readShellEnv(shellPath, ['-ic', 'env -0']); @@ -187,7 +214,11 @@ export async function resolveInteractiveShellEnv( } catch (interactiveError) { const interactiveMessage = interactiveError instanceof Error ? interactiveError.message : String(interactiveError); - logger.warn(`Failed to resolve interactive shell env: ${interactiveMessage}`); + logger.warn( + `Failed to resolve shell env after login and interactive probes${formatShellEnvSource( + options + )}: login=${loginMessage}; interactive=${interactiveMessage}` + ); rememberShellEnvFailure(interactiveMessage); emitProgress(options, 'shell-env-fallback', 'Using current process environment...'); return {}; @@ -222,6 +253,10 @@ export async function resolveInteractiveShellEnvBestEffort( const fallbackEnv = options.fallbackEnv ?? {}; const timeoutMs = Math.max(0, options.timeoutMs ?? SHELL_ENV_BEST_EFFORT_TIMEOUT_MS); const startedAt = Date.now(); + if (options.background === false) { + emitProgress(options, 'shell-env-best-effort-fallback', 'Using fallback shell environment...'); + return fallbackEnv; + } if (!shellEnvResolvePromise && startedAt < shellEnvFailureCooldownUntil) { const retryInMs = Math.max(0, shellEnvFailureCooldownUntil - startedAt); emitProgress( diff --git a/test/main/features/codex-runtime-installer/CodexRuntimeInstallerService.test.ts b/test/main/features/codex-runtime-installer/CodexRuntimeInstallerService.test.ts index 41af6fd5..602f3463 100644 --- a/test/main/features/codex-runtime-installer/CodexRuntimeInstallerService.test.ts +++ b/test/main/features/codex-runtime-installer/CodexRuntimeInstallerService.test.ts @@ -2,8 +2,8 @@ import { createHash } from 'crypto'; import { chmod, mkdir, mkdtemp, rm, writeFile } from 'fs/promises'; import os from 'os'; import path from 'path'; -import { gzipSync } from 'zlib'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { gzipSync } from 'zlib'; const execCliMock = vi.hoisted(() => vi.fn()); const buildMergedCliPathMock = vi.hoisted(() => vi.fn(() => process.env.PATH ?? '')); diff --git a/test/main/services/team/ClaudeBinaryResolver.test.ts b/test/main/services/team/ClaudeBinaryResolver.test.ts index d8cd7eff..b342a6e0 100644 --- a/test/main/services/team/ClaudeBinaryResolver.test.ts +++ b/test/main/services/team/ClaudeBinaryResolver.test.ts @@ -1,12 +1,15 @@ // @vitest-environment node -import type { PathLike } from 'fs'; import * as path from 'path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { PathLike } from 'fs'; + const mockBuildMergedCliPath = vi.fn<(binaryPath: string | null) => string>(); const mockGetShellPreferredHome = vi.fn<() => string>(); const mockGetClaudeBasePath = vi.fn<() => string>(); -const mockResolveInteractiveShellEnv = vi.fn<() => Promise>(); +const mockResolveInteractiveShellEnvBestEffort = vi.fn< + (options?: unknown) => Promise +>(); const mockGetConfiguredCliFlavor = vi.fn<() => 'claude' | 'agent_teams_orchestrator'>(); const mockGetDoctorInvokedCandidates = vi.fn<(commandName: string) => Promise>(); @@ -19,7 +22,8 @@ vi.mock('@main/utils/cliPathMerge', () => ({ vi.mock('@main/utils/shellEnv', () => ({ getShellPreferredHome: () => mockGetShellPreferredHome(), - resolveInteractiveShellEnv: () => mockResolveInteractiveShellEnv(), + resolveInteractiveShellEnvBestEffort: (options?: unknown) => + mockResolveInteractiveShellEnvBestEffort(options), })); vi.mock('@main/utils/pathDecoder', () => ({ @@ -62,7 +66,7 @@ describe('ClaudeBinaryResolver', () => { mockBuildMergedCliPath.mockReturnValue(['/usr/local/bin', '/usr/bin'].join(path.delimiter)); mockGetShellPreferredHome.mockReturnValue('/Users/tester'); mockGetClaudeBasePath.mockReturnValue('/Users/tester/.claude'); - mockResolveInteractiveShellEnv.mockResolvedValue({}); + mockResolveInteractiveShellEnvBestEffort.mockResolvedValue({}); mockGetConfiguredCliFlavor.mockReturnValue('agent_teams_orchestrator'); mockGetDoctorInvokedCandidates.mockResolvedValue([]); Object.defineProperty(process, 'platform', { @@ -139,7 +143,9 @@ describe('ClaudeBinaryResolver', () => { it('does not wait for shell env before using an explicit absolute runtime override', async () => { const expectedBinary = '/Users/belief/dev/projects/claude/agent_teams_orchestrator/cli-dev'; process.env.CLAUDE_AGENT_TEAMS_ORCHESTRATOR_CLI_PATH = expectedBinary; - mockResolveInteractiveShellEnv.mockRejectedValue(new Error('shell env should not be needed')); + mockResolveInteractiveShellEnvBestEffort.mockRejectedValue( + new Error('shell env should not be needed') + ); accessMock.mockImplementation((filePath) => { if (filePath === expectedBinary) { @@ -152,7 +158,7 @@ describe('ClaudeBinaryResolver', () => { ClaudeBinaryResolver.clearCache(); await expect(ClaudeBinaryResolver.resolve()).resolves.toBe(expectedBinary); - expect(mockResolveInteractiveShellEnv).not.toHaveBeenCalled(); + expect(mockResolveInteractiveShellEnvBestEffort).not.toHaveBeenCalled(); }); it('resolves extensionless Windows explicit overrides to a real executable file first', async () => { @@ -214,6 +220,13 @@ describe('ClaudeBinaryResolver', () => { ClaudeBinaryResolver.clearCache(); await expect(ClaudeBinaryResolver.resolve()).resolves.toBe(expectedBinary); + expect(mockResolveInteractiveShellEnvBestEffort).toHaveBeenCalledWith( + expect.objectContaining({ + timeoutMs: 1_500, + fallbackEnv: process.env, + background: false, + }) + ); expect(accessMock).toHaveBeenCalledWith(expectedBinary, 1); }); diff --git a/test/main/utils/runtimePathBinaryResolver.test.ts b/test/main/utils/runtimePathBinaryResolver.test.ts new file mode 100644 index 00000000..c91381dd --- /dev/null +++ b/test/main/utils/runtimePathBinaryResolver.test.ts @@ -0,0 +1,99 @@ +import { chmod, mkdir, mkdtemp, rm, writeFile } from 'fs/promises'; +import os from 'os'; +import path from 'path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const buildMergedCliPathMock = vi.hoisted(() => vi.fn(() => '')); +const getCachedShellEnvMock = vi.hoisted(() => vi.fn<() => NodeJS.ProcessEnv | null>(() => null)); + +vi.mock('@main/utils/cliPathMerge', () => ({ + buildMergedCliPath: buildMergedCliPathMock, +})); + +vi.mock('@main/utils/shellEnv', () => ({ + getCachedShellEnv: getCachedShellEnvMock, +})); + +import { + collectRuntimePathBinaryCandidates, + findFirstRuntimePathBinaryCandidate, + RUNTIME_PATH_SHELL_ENV_TIMEOUT_MS, +} from '@main/utils/runtimePathBinaryResolver'; + +describe('runtimePathBinaryResolver', () => { + let tempRoot: string | null = null; + let originalPath: string | undefined; + + beforeEach(async () => { + tempRoot = await mkdtemp(path.join(os.tmpdir(), 'runtime-path-binary-resolver-')); + originalPath = process.env.PATH; + process.env.PATH = ''; + buildMergedCliPathMock.mockReset(); + buildMergedCliPathMock.mockReturnValue(''); + getCachedShellEnvMock.mockReset(); + getCachedShellEnvMock.mockReturnValue(null); + }); + + afterEach(async () => { + if (originalPath === undefined) { + delete process.env.PATH; + } else { + process.env.PATH = originalPath; + } + if (tempRoot) { + await rm(tempRoot, { recursive: true, force: true }); + tempRoot = null; + } + }); + + async function createExecutable(dirName: string, name: string): Promise { + const binaryPath = path.join(tempRoot!, dirName, name); + await mkdir(path.dirname(binaryPath), { recursive: true }); + await writeFile(binaryPath, 'binary'); + if (process.platform !== 'win32') { + await chmod(binaryPath, 0o755); + } + return binaryPath; + } + + it('prefers explicit env sources before cached and fallback PATH entries', async () => { + const explicitBinary = await createExecutable('explicit-bin', 'tool'); + const cachedBinary = await createExecutable('cached-bin', 'tool'); + const fallbackBinary = await createExecutable('fallback-bin', 'tool'); + getCachedShellEnvMock.mockReturnValue({ PATH: path.dirname(cachedBinary) }); + buildMergedCliPathMock.mockReturnValue(path.dirname(fallbackBinary)); + + expect( + findFirstRuntimePathBinaryCandidate({ + executableNames: ['tool'], + additionalEnvSources: [{ PATH: path.dirname(explicitBinary) }], + }) + ).toBe(explicitBinary); + }); + + it('keeps extra candidates before fallback PATH entries and filters missing files', async () => { + const extraBinary = await createExecutable('extra-bin', 'tool'); + const fallbackBinary = await createExecutable('fallback-bin', 'tool'); + buildMergedCliPathMock.mockReturnValue(path.dirname(fallbackBinary)); + + expect( + collectRuntimePathBinaryCandidates({ + executableNames: ['tool'], + extraCandidates: [path.join(tempRoot!, 'missing', 'tool'), extraBinary], + }) + ).toEqual([extraBinary, fallbackBinary]); + }); + + it('can skip fallback PATH entries for staged shell-env lookup', async () => { + const fallbackBinary = await createExecutable('fallback-bin', 'tool'); + buildMergedCliPathMock.mockReturnValue(path.dirname(fallbackBinary)); + + expect( + collectRuntimePathBinaryCandidates({ + executableNames: ['tool'], + includeFallbackPathEntries: false, + }) + ).toEqual([]); + expect(RUNTIME_PATH_SHELL_ENV_TIMEOUT_MS).toBe(1_500); + }); +}); diff --git a/test/main/utils/shellEnv.integration.test.ts b/test/main/utils/shellEnv.integration.test.ts index 183b9500..5c507b36 100644 --- a/test/main/utils/shellEnv.integration.test.ts +++ b/test/main/utils/shellEnv.integration.test.ts @@ -1,16 +1,14 @@ // @vitest-environment node -import { chmod, mkdtemp, readFile, rm, writeFile } from 'fs/promises'; -import { tmpdir } from 'os'; -import path from 'path'; - -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; - import { clearShellEnvCache, getCachedShellEnv, resolveInteractiveShellEnv, resolveInteractiveShellEnvBestEffort, } from '@main/utils/shellEnv'; +import { chmod, mkdtemp, readFile, rm, writeFile } from 'fs/promises'; +import { tmpdir } from 'os'; +import path from 'path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const describePosix = process.platform === 'win32' ? describe.skip : describe; @@ -134,6 +132,37 @@ setTimeout(() => { }); }); + it('returns fallback without spawning shell when background resolution is disabled', async () => { + const invocationFile = path.join(tempDir, 'no-background-invocations.log'); + process.env.FAKE_SHELL_INVOCATIONS = invocationFile; + const fakeShell = await createFakeShell( + tempDir, + 'no-background-shell.js', + envWriterSource(` +const fs = require('fs'); +fs.appendFileSync(process.env.FAKE_SHELL_INVOCATIONS, 'spawned\\n'); +writeEnv({ + PATH: '/should-not-run/bin', + HOME: '/should-not-run-home', +}); +`) + ); + process.env.SHELL = fakeShell; + + const startedAt = Date.now(); + const env = await resolveInteractiveShellEnvBestEffort({ + timeoutMs: 25, + fallbackEnv: { PATH: 'FALLBACK_PATH', HOME: 'FALLBACK_HOME' }, + background: false, + }); + const elapsedMs = Date.now() - startedAt; + + expect(elapsedMs).toBeLessThan(150); + expect(env).toMatchObject({ PATH: 'FALLBACK_PATH', HOME: 'FALLBACK_HOME' }); + expect(getCachedShellEnv()).toBeNull(); + await expect(readFile(invocationFile, 'utf8')).rejects.toMatchObject({ code: 'ENOENT' }); + }); + it('falls back from a failed login shell process to a successful interactive shell process', async () => { const fakeShell = await createFakeShell( tempDir, @@ -154,10 +183,7 @@ writeEnv({ PATH: '/interactive-real/bin:/usr/bin', HOME: '/interactive-home', }); - expect(console.warn).toHaveBeenCalledWith( - '[Utils:shellEnv]', - 'Failed to resolve login shell env: shell env command exited with code 42' - ); + expect(console.warn).not.toHaveBeenCalled(); vi.mocked(console.warn).mockClear(); expect(getCachedShellEnv()).toMatchObject({ PATH: '/interactive-real/bin:/usr/bin', diff --git a/test/main/utils/shellEnv.test.ts b/test/main/utils/shellEnv.test.ts index aa26b08f..13d4da47 100644 --- a/test/main/utils/shellEnv.test.ts +++ b/test/main/utils/shellEnv.test.ts @@ -1,6 +1,5 @@ // @vitest-environment node import { EventEmitter } from 'events'; - import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const hoisted = vi.hoisted(() => ({ @@ -127,6 +126,48 @@ describe('shellEnv', () => { ); }); + it('adds a sanitized source label to strict shell failure diagnostics', async () => { + const children: MockChildProcess[] = []; + hoisted.spawn.mockImplementation(() => { + const child = createChild(); + children.push(child); + const attempt = children.length; + queueMicrotask(() => { + emitError(child, attempt === 1 ? 'login blocked' : 'interactive blocked'); + }); + return child; + }); + + const progress = vi.fn(); + const shellEnv = await importShellEnv(); + + await expect( + shellEnv.resolveInteractiveShellEnv({ + source: ' mcp node/runtime ', + onProgress: progress, + }) + ).resolves.toEqual({}); + + expect(progress).toHaveBeenCalledWith({ + phase: 'shell-env-login', + message: 'Reading login shell environment...', + source: 'mcp_node_runtime', + }); + expect(progress).toHaveBeenCalledWith({ + phase: 'shell-env-interactive', + message: 'Trying interactive shell environment...', + source: 'mcp_node_runtime', + }); + expect(progress).toHaveBeenCalledWith({ + phase: 'shell-env-fallback', + message: 'Using current process environment...', + source: 'mcp_node_runtime', + }); + expect(hoisted.loggerWarn).toHaveBeenCalledWith( + 'Failed to resolve shell env after login and interactive probes source=mcp_node_runtime: login=login blocked; interactive=interactive blocked' + ); + }); + it('returns fallback on soft timeout without caching it, then caches background success', async () => { const children: MockChildProcess[] = []; hoisted.spawn.mockImplementation(() => { @@ -259,6 +300,22 @@ describe('shellEnv', () => { }); }); + it('can return fallback without starting a background shell probe', async () => { + const shellEnv = await importShellEnv(); + const fallbackEnv = { PATH: '/fallback/no-background' }; + + await expect( + shellEnv.resolveInteractiveShellEnvBestEffort({ + timeoutMs: 0, + fallbackEnv, + background: false, + }) + ).resolves.toBe(fallbackEnv); + + expect(hoisted.spawn).not.toHaveBeenCalled(); + expect(shellEnv.getCachedShellEnv()).toBeNull(); + }); + it('keeps resolving in the background through the strict interactive fallback', async () => { const children: MockChildProcess[] = []; hoisted.spawn.mockImplementation(() => { @@ -315,9 +372,7 @@ describe('shellEnv', () => { HOME: '/Users/tester', }); expect(hoisted.spawn).toHaveBeenCalledTimes(2); - expect(hoisted.loggerWarn).toHaveBeenCalledWith( - 'Failed to resolve login shell env: shell env command exited with code 42' - ); + expect(hoisted.loggerWarn).not.toHaveBeenCalled(); }); it('coalesces concurrent best-effort calls behind one shell process', async () => { @@ -443,6 +498,9 @@ describe('shellEnv', () => { await expect(result).resolves.toMatchObject({ PATH: '/fallback/stuck' }); expect(children[1].kill).toHaveBeenCalledWith(); expect(shellEnv.getCachedShellEnv()).toBeNull(); + expect(hoisted.loggerWarn).toHaveBeenCalledWith( + 'Failed to resolve shell env after login and interactive probes: login=shell env resolve timeout; interactive=shell env resolve timeout' + ); await vi.advanceTimersByTimeAsync(3_000);