From 3319a01d6a288dedfc75c18e6a80134cac673085 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Fri, 10 Apr 2026 13:51:30 -0700 Subject: [PATCH 1/4] implement json mode --- src/common/telemetry/constants.ts | 21 + src/managers/common/nativePythonFinder.ts | 394 ++++++++++++++-- .../nativePythonFinder.jsonCli.unit.test.ts | 437 ++++++++++++++++++ 3 files changed, 819 insertions(+), 33 deletions(-) create mode 100644 src/test/managers/common/nativePythonFinder.jsonCli.unit.test.ts diff --git a/src/common/telemetry/constants.ts b/src/common/telemetry/constants.ts index 29d7aa8a..950143de 100644 --- a/src/common/telemetry/constants.ts +++ b/src/common/telemetry/constants.ts @@ -113,6 +113,15 @@ export enum EventNames { * - errorType: string (classified error category, on failure only) */ MANAGER_LAZY_INIT = 'MANAGER.LAZY_INIT', + /** + * Telemetry event fired when the JSON CLI fallback is used for environment discovery. + * Triggered when the PET JSON-RPC server mode is exhausted after all restart attempts. + * Properties: + * - operation: 'refresh' | 'resolve' + * - result: 'success' | 'error' + * - duration: number (milliseconds taken for the CLI operation) + */ + PET_JSON_CLI_FALLBACK = 'PET.JSON_CLI_FALLBACK', } // Map all events to their properties @@ -403,4 +412,16 @@ export interface IEventNamePropertyMapping { toolSource: string; errorType?: string; }; + + /* __GDPR__ + "pet.json_cli_fallback": { + "operation": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "StellaHuang95" }, + "result": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "StellaHuang95" }, + "": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "owner": "StellaHuang95" } + } + */ + [EventNames.PET_JSON_CLI_FALLBACK]: { + operation: 'refresh' | 'resolve'; + result: 'success' | 'error'; + }; } diff --git a/src/managers/common/nativePythonFinder.ts b/src/managers/common/nativePythonFinder.ts index d5a2423a..1d5b4148 100644 --- a/src/managers/common/nativePythonFinder.ts +++ b/src/managers/common/nativePythonFinder.ts @@ -9,6 +9,9 @@ import { spawnProcess } from '../../common/childProcess.apis'; import { ENVS_EXTENSION_ID, PYTHON_EXTENSION_ID } from '../../common/constants'; import { getExtension } from '../../common/extension.apis'; import { traceError, traceVerbose, traceWarn } from '../../common/logging'; +import { StopWatch } from '../../common/stopWatch'; +import { EventNames } from '../../common/telemetry/constants'; +import { sendTelemetryEvent } from '../../common/telemetry/sender'; import { untildify, untildifyArray } from '../../common/utils/pathUtils'; import { isWindows } from '../../common/utils/platformUtils'; import { createRunningWorkerPool, WorkerPool } from '../../common/utils/workerPool'; @@ -21,6 +24,9 @@ const MAX_CONFIGURE_TIMEOUT_MS = 60_000; // Max configure timeout after retries const REFRESH_TIMEOUT_MS = 30_000; // 30 seconds for full refresh (with 1 retry = 60s max) const RESOLVE_TIMEOUT_MS = 30_000; // 30 seconds for single resolve +// CLI fallback timeout: generous budget since it's a full process spawn doing a full scan +const CLI_FALLBACK_TIMEOUT_MS = 120_000; // 2 minutes + // Restart/recovery constants const MAX_RESTART_ATTEMPTS = 3; const RESTART_BACKOFF_BASE_MS = 1_000; // 1 second base, exponential: 1s, 2s, 4s @@ -245,28 +251,37 @@ class NativePythonFinderImpl implements NativePythonFinder { } public async resolve(executable: string): Promise { - await this.ensureProcessRunning(); try { - await this.configure(); - const environment = await sendRequestWithTimeout( - this.connection, - 'resolve', - { executable }, - RESOLVE_TIMEOUT_MS, - ); - - this.outputChannel.info(`Resolved Python Environment ${environment.executable}`); - // Reset restart attempts on successful request - this.restartAttempts = 0; - return environment; + await this.ensureProcessRunning(); + try { + await this.configure(); + const environment = await sendRequestWithTimeout( + this.connection, + 'resolve', + { executable }, + RESOLVE_TIMEOUT_MS, + ); + + this.outputChannel.info(`Resolved Python Environment ${environment.executable}`); + // Reset restart attempts on successful request + this.restartAttempts = 0; + return environment; + } catch (ex) { + // On resolve timeout or connection error (not configure — configure handles its own timeout), + // kill the hung process so next request triggers restart + if ((ex instanceof RpcTimeoutError && ex.method !== 'configure') || ex instanceof rpc.ConnectionError) { + const reason = ex instanceof rpc.ConnectionError ? 'crashed' : 'timed out'; + this.outputChannel.warn(`[pet] Resolve request ${reason}, killing process for restart`); + this.killProcess(); + this.processExited = true; + } + throw ex; + } } catch (ex) { - // On resolve timeout or connection error (not configure — configure handles its own timeout), - // kill the hung process so next request triggers restart - if ((ex instanceof RpcTimeoutError && ex.method !== 'configure') || ex instanceof rpc.ConnectionError) { - const reason = ex instanceof rpc.ConnectionError ? 'crashed' : 'timed out'; - this.outputChannel.warn(`[pet] Resolve request ${reason}, killing process for restart`); - this.killProcess(); - this.processExited = true; + // If the server mode is fully exhausted, fall back to the CLI JSON mode + if (this.isServerExhausted()) { + this.outputChannel.warn('[pet] Server mode exhausted, falling back to JSON CLI for resolve'); + return this.resolveViaJsonCli(executable); } throw ex; } @@ -592,12 +607,20 @@ class NativePythonFinderImpl implements NativePythonFinder { // Final attempt failed this.outputChannel.error(`[pet] Refresh failed after ${MAX_REFRESH_RETRIES + 1} attempts`); } - // Non-retryable errors or final attempt - rethrow + // Non-timeout errors or final timeout — check if server is fully exhausted + if (this.isServerExhausted()) { + this.outputChannel.warn('[pet] Server mode exhausted, falling back to JSON CLI for refresh'); + return this.refreshViaJsonCli(options); + } throw ex; } } // Should not reach here, but TypeScript needs this + if (this.isServerExhausted()) { + this.outputChannel.warn('[pet] Server mode exhausted, falling back to JSON CLI for refresh (final)'); + return this.refreshViaJsonCli(options); + } throw lastError; } @@ -680,17 +703,7 @@ class NativePythonFinderImpl implements NativePythonFinder { * Must be invoked when ever there are changes to any data related to the configuration details. */ private async configure() { - // Get all extra search paths including legacy settings and new searchPaths - const extraSearchPaths = await getAllExtraSearchPaths(); - - const options: ConfigurationOptions = { - workspaceDirectories: this.api.getPythonProjects().map((item) => item.uri.fsPath), - environmentDirectories: extraSearchPaths, - condaExecutable: getPythonSettingAndUntildify('condaPath'), - pipenvExecutable: getPythonSettingAndUntildify('pipenvPath'), - poetryExecutable: getPythonSettingAndUntildify('poetryPath'), - cacheDirectory: this.cacheDirectory?.fsPath, - }; + const options = await this.buildConfigurationOptions(); // No need to send a configuration request if there are no changes. if (this.lastConfiguration && this.configurationEquals(options, this.lastConfiguration)) { this.outputChannel.debug('[pet] configure: No changes detected, skipping configuration update.'); @@ -735,6 +748,213 @@ class NativePythonFinderImpl implements NativePythonFinder { } } + /** + * Builds the current ConfigurationOptions from VS Code settings and the active workspace. + * Extracted from configure() so the CLI fallback can build the same config. + */ + private async buildConfigurationOptions(): Promise { + // Get all extra search paths including legacy settings and new searchPaths + const extraSearchPaths = await getAllExtraSearchPaths(); + return { + workspaceDirectories: this.api.getPythonProjects().map((item) => item.uri.fsPath), + environmentDirectories: extraSearchPaths, + condaExecutable: getPythonSettingAndUntildify('condaPath'), + pipenvExecutable: getPythonSettingAndUntildify('pipenvPath'), + poetryExecutable: getPythonSettingAndUntildify('poetryPath'), + cacheDirectory: this.cacheDirectory?.fsPath, + }; + } + + /** + * Returns true when all server restart attempts have been exhausted. + * Used to decide whether to fall back to CLI mode. + */ + private isServerExhausted(): boolean { + return this.restartAttempts >= MAX_RESTART_ATTEMPTS && (this.startFailed || this.processExited); + } + + /** + * Spawns the PET binary with the given args and collects its stdout. + * Uses direct spawn (not shell) to avoid injection risks from user-supplied paths. + * Kills the process after `timeoutMs` to prevent hangs. + * + * @param args Arguments to pass to the PET binary. + * @param timeoutMs Maximum time to wait for the process to complete. + * @returns The stdout string. + */ + private runPetCliProcess(args: string[], timeoutMs: number): Promise { + return new Promise((resolve, reject) => { + const proc = spawnProcess(this.toolPath, args, { stdio: 'pipe' }); + let stdout = ''; + + const timer = setTimeout(() => { + try { + proc.kill('SIGTERM'); + // Force kill after a short grace period if still running + setTimeout(() => { + if (proc.exitCode === null) { + proc.kill('SIGKILL'); + } + }, 500); + } catch { + // Ignore kill errors + } + reject(new Error(`PET CLI process timed out after ${timeoutMs}ms`)); + }, timeoutMs); + + proc.stdout.on('data', (data: Buffer) => { + stdout += data.toString(); + }); + proc.stderr.on('data', (data: Buffer) => { + // PET writes diagnostics/logs to stderr in --json mode; surface them as debug + this.outputChannel.debug(`[pet CLI] ${data.toString().trimEnd()}`); + }); + proc.on('close', (code) => { + clearTimeout(timer); + // If the process failed and produced no output, reject so caller gets a clear error + if (code !== 0 && stdout.trim().length === 0) { + reject(new Error(`PET CLI process exited with code ${code}`)); + return; + } + resolve(stdout); + }); + proc.on('error', (err) => { + clearTimeout(timer); + reject(err); + }); + }); + } + + /** + * Fallback environment refresh using `pet find --json`. + * Invoked when the JSON-RPC server mode is exhausted after all restart attempts. + * Spawns PET as a one-shot subprocess and parses the JSON output. + * + * @param options Optional kind filter or URI search paths (same semantics as refresh()). + * @returns NativeInfo[] containing managers and environments, same as server mode. + */ + private async refreshViaJsonCli(options?: NativePythonEnvironmentKind | Uri[]): Promise { + const config = await this.buildConfigurationOptions(); + // venvFolders must be included explicitly as search paths when options is Uri[], + // mirroring getRefreshOptions() server-mode behaviour (searchPaths may override environmentDirectories). + const venvFolders = getPythonSettingAndUntildify('venvFolders') ?? []; + const args = buildFindCliArgs(config, options, venvFolders); + + this.outputChannel.info(`[pet] JSON CLI fallback refresh: ${this.toolPath} ${args.join(' ')}`); + const stopWatch = new StopWatch(); + + let stdout: string; + try { + stdout = await this.runPetCliProcess(args, CLI_FALLBACK_TIMEOUT_MS); + } catch (ex) { + sendTelemetryEvent(EventNames.PET_JSON_CLI_FALLBACK, stopWatch.elapsedTime, { + operation: 'refresh', + result: 'error', + }); + this.outputChannel.error('[pet] JSON CLI fallback refresh failed:', ex); + throw ex; + } + + let parsed: { managers: NativeEnvManagerInfo[]; environments: NativeEnvInfo[] }; + try { + parsed = parseRefreshCliOutput(stdout); + } catch { + sendTelemetryEvent(EventNames.PET_JSON_CLI_FALLBACK, stopWatch.elapsedTime, { + operation: 'refresh', + result: 'error', + }); + this.outputChannel.error('[pet] JSON CLI fallback: Failed to parse find output:', stdout.slice(0, 500)); + throw new Error('Failed to parse PET find --json output'); + } + + const nativeInfo: NativeInfo[] = []; + + for (const manager of parsed.managers ?? []) { + this.outputChannel.info(`[pet CLI] Discovered manager: (${manager.tool}) ${manager.executable}`); + nativeInfo.push(manager); + } + + for (const env of parsed.environments ?? []) { + if (env.executable && (!env.version || !env.prefix)) { + // Environment has an executable but incomplete metadata — resolve individually + try { + const resolved = await this.resolveViaJsonCli(env.executable); + this.outputChannel.info(`[pet CLI] Resolved env: ${resolved.executable}`); + nativeInfo.push(resolved); + } catch { + // If resolve fails, still include the partial env so nothing is silently dropped + this.outputChannel.warn( + `[pet CLI] Could not resolve incomplete env, using partial data: ${env.executable}`, + ); + nativeInfo.push(env); + } + } else { + this.outputChannel.info(`[pet CLI] Discovered env: ${env.executable ?? env.prefix}`); + nativeInfo.push(env); + } + } + + sendTelemetryEvent(EventNames.PET_JSON_CLI_FALLBACK, stopWatch.elapsedTime, { + operation: 'refresh', + result: 'success', + }); + return nativeInfo; + } + + /** + * Fallback environment resolution using `pet resolve --json`. + * Invoked when the JSON-RPC server mode is exhausted after all restart attempts. + * + * @param executable Path to the Python executable to resolve. + * @returns The resolved NativeEnvInfo. + * @throws Error if PET cannot identify the environment or if the output cannot be parsed. + */ + private async resolveViaJsonCli(executable: string): Promise { + const args = ['resolve', executable, '--json']; + if (this.cacheDirectory) { + args.push('--cache-directory', this.cacheDirectory.fsPath); + } + + this.outputChannel.info(`[pet] JSON CLI fallback resolve: ${this.toolPath} ${args.join(' ')}`); + const stopWatch = new StopWatch(); + + let stdout: string; + try { + stdout = await this.runPetCliProcess(args, CLI_FALLBACK_TIMEOUT_MS); + } catch (ex) { + sendTelemetryEvent(EventNames.PET_JSON_CLI_FALLBACK, stopWatch.elapsedTime, { + operation: 'resolve', + result: 'error', + }); + throw ex; + } + + let parsed: NativeEnvInfo; + try { + parsed = parseResolveCliOutput(stdout.trim(), executable); + } catch (ex) { + sendTelemetryEvent(EventNames.PET_JSON_CLI_FALLBACK, stopWatch.elapsedTime, { + operation: 'resolve', + result: 'error', + }); + if (ex instanceof SyntaxError) { + this.outputChannel.error( + '[pet] JSON CLI fallback: Failed to parse resolve output:', + stdout.slice(0, 200), + ); + throw new Error(`Failed to parse PET resolve --json output for ${executable}`); + } + // "not found" (null) or other parse error + throw ex; + } + + sendTelemetryEvent(EventNames.PET_JSON_CLI_FALLBACK, stopWatch.elapsedTime, { + operation: 'resolve', + result: 'success', + }); + return parsed; + } + /** * Compares two ConfigurationOptions objects for equality. * Uses property-by-property comparison to avoid issues with JSON.stringify @@ -776,7 +996,7 @@ class NativePythonFinderImpl implements NativePythonFinder { } } -type ConfigurationOptions = { +export type ConfigurationOptions = { workspaceDirectories: string[]; environmentDirectories: string[]; condaExecutable: string | undefined; @@ -784,6 +1004,114 @@ type ConfigurationOptions = { poetryExecutable: string | undefined; cacheDirectory?: string; }; + +/** + * Parses the stdout of `pet find --json` into a structured result. + * Returns `{ managers, environments }` arrays (each may be empty). + * + * @param stdout Raw stdout from `pet find --json`. + * @returns Parsed result object. + * @throws SyntaxError if `stdout` is not valid JSON or not the expected object shape. + */ +export function parseRefreshCliOutput(stdout: string): { + managers: NativeEnvManagerInfo[]; + environments: NativeEnvInfo[]; +} { + // May throw SyntaxError on malformed JSON — callers must handle + const parsed = JSON.parse(stdout); + if (typeof parsed !== 'object' || parsed === null) { + throw new SyntaxError('PET find --json output is not a JSON object'); + } + return { + managers: Array.isArray(parsed.managers) ? parsed.managers : [], + environments: Array.isArray(parsed.environments) ? parsed.environments : [], + }; +} + +/** + * Parses the stdout of `pet resolve --json` into a single environment info object. + * + * @param stdout Raw stdout from `pet resolve --json` (trimmed). + * @param executable The executable that was resolved (used in error messages). + * @returns The parsed `NativeEnvInfo`. + * @throws Error if `stdout` is `"null"` (environment not found) or malformed JSON. + */ +export function parseResolveCliOutput(stdout: string, executable: string): NativeEnvInfo { + // May throw SyntaxError on malformed JSON — callers must handle + const parsed: NativeEnvInfo | null = JSON.parse(stdout); + if (parsed === null) { + throw new Error(`PET could not identify environment for executable: ${executable}`); + } + return parsed; +} + +/** + * Builds the CLI arguments array for a `pet find --json` invocation. + * This is exported for testability. + * + * @param config The configuration options (workspace dirs, tool paths, cache dir, env dirs). + * @param options Optional refresh options: a kind filter string or an array of URIs to search. + * @param venvFolders Additional virtual environment folder paths to include when searching + * URI-based paths (needed because searchPaths may override environmentDirectories in PET). + * @returns The args array to pass to the PET binary (after 'find --json'). + */ +export function buildFindCliArgs( + config: ConfigurationOptions, + options?: NativePythonEnvironmentKind | Uri[], + venvFolders: string[] = [], +): string[] { + const args: string[] = ['find', '--json']; + + if (options) { + if (typeof options === 'string') { + // NativePythonEnvironmentKind — filter by environment kind. + // In server mode, `build_refresh_config` keeps the configured workspace dirs when + // search_kind is set, so workspace-scoped envs of that kind (e.g. Venv) are found. + // Mirror that here by passing workspace dirs as positional search paths. + args.push('--kind', options); + for (const dir of config.workspaceDirectories) { + args.push(dir); + } + } else if (Array.isArray(options)) { + // Uri[] — these become the positional search paths (overriding workspace dirs). + // In server mode, `build_refresh_config` sets search_scope = Workspace, which causes + // find_and_report_envs to skip all global discovery phases (locators, PATH, global venvs) + // and only search the provided paths. Mirror that with --workspace. + args.push('--workspace'); + for (const uri of options) { + args.push(uri.fsPath); + } + for (const folder of venvFolders) { + args.push(folder); + } + } + } else { + // No options: pass workspace directories as positional search paths + for (const dir of config.workspaceDirectories) { + args.push(dir); + } + } + + // Always forward configuration flags + if (config.cacheDirectory) { + args.push('--cache-directory', config.cacheDirectory); + } + if (config.condaExecutable) { + args.push('--conda-executable', config.condaExecutable); + } + if (config.pipenvExecutable) { + args.push('--pipenv-executable', config.pipenvExecutable); + } + if (config.poetryExecutable) { + args.push('--poetry-executable', config.poetryExecutable); + } + if (config.environmentDirectories.length > 0) { + // PET accepts comma-separated dirs for --environment-directories + args.push('--environment-directories', config.environmentDirectories.join(',')); + } + + return args; +} /** * Gets all custom virtual environment locations to look for environments from the legacy python settings (venvPath, venvFolders). */ diff --git a/src/test/managers/common/nativePythonFinder.jsonCli.unit.test.ts b/src/test/managers/common/nativePythonFinder.jsonCli.unit.test.ts new file mode 100644 index 00000000..ceb468b9 --- /dev/null +++ b/src/test/managers/common/nativePythonFinder.jsonCli.unit.test.ts @@ -0,0 +1,437 @@ +import assert from 'node:assert'; +import { Uri } from 'vscode'; +import { + buildFindCliArgs, + ConfigurationOptions, + NativeEnvInfo, + NativeEnvManagerInfo, + NativePythonEnvironmentKind, + parseRefreshCliOutput, + parseResolveCliOutput, +} from '../../../managers/common/nativePythonFinder'; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeConfig(overrides: Partial = {}): ConfigurationOptions { + return { + workspaceDirectories: [], + environmentDirectories: [], + condaExecutable: undefined, + pipenvExecutable: undefined, + poetryExecutable: undefined, + cacheDirectory: undefined, + ...overrides, + }; +} + +// --------------------------------------------------------------------------- +// buildFindCliArgs — no options (search everything) +// --------------------------------------------------------------------------- + +suite('buildFindCliArgs — no options', () => { + test('starts with ["find", "--json"]', () => { + const args = buildFindCliArgs(makeConfig()); + assert.ok(args[0] === 'find' && args[1] === '--json', `Expected ["find","--json"], got [${args.slice(0, 2)}]`); + }); + + test('includes workspace directories as positional args', () => { + const config = makeConfig({ workspaceDirectories: ['/home/user/project', '/home/user/other'] }); + const args = buildFindCliArgs(config); + assert.ok(args.includes('/home/user/project'), 'Should include first workspace dir'); + assert.ok(args.includes('/home/user/other'), 'Should include second workspace dir'); + }); + + test('does NOT include --kind flag', () => { + const args = buildFindCliArgs(makeConfig({ workspaceDirectories: ['/mydir'] })); + assert.ok(!args.includes('--kind'), 'Should not include --kind'); + }); + + test('produces only ["find","--json"] when all config fields are empty', () => { + const args = buildFindCliArgs(makeConfig()); + assert.deepStrictEqual(args, ['find', '--json']); + }); +}); + +// --------------------------------------------------------------------------- +// buildFindCliArgs — NativePythonEnvironmentKind (string) options +// --------------------------------------------------------------------------- + +suite('buildFindCliArgs — kind filter', () => { + test('appends --kind with the kind value', () => { + const args = buildFindCliArgs(makeConfig(), NativePythonEnvironmentKind.conda); + assert.ok(args.includes('--kind'), 'Should include --kind flag'); + const idx = args.indexOf('--kind'); + assert.strictEqual(args[idx + 1], 'Conda', 'Kind value should be Conda'); + }); + + test('includes workspace dirs as positional args when kind is set (mirrors server mode)', () => { + // In server mode, build_refresh_config keeps the configured workspace dirs when + // search_kind is set so workspace-scoped envs of that kind (e.g. Venv) are found. + const config = makeConfig({ workspaceDirectories: ['/mydir', '/otherdir'] }); + const args = buildFindCliArgs(config, NativePythonEnvironmentKind.venv); + assert.ok(args.includes('/mydir'), 'Should include first workspace dir as positional'); + assert.ok(args.includes('/otherdir'), 'Should include second workspace dir as positional'); + }); + + test('does NOT add --workspace flag when kind is set', () => { + const args = buildFindCliArgs(makeConfig(), NativePythonEnvironmentKind.conda); + assert.ok(!args.includes('--workspace'), 'Should not include --workspace for kind filter'); + }); + + test('works for all kind values (spot check)', () => { + const kinds: NativePythonEnvironmentKind[] = [ + NativePythonEnvironmentKind.conda, + NativePythonEnvironmentKind.homebrew, + NativePythonEnvironmentKind.pipenv, + NativePythonEnvironmentKind.poetry, + NativePythonEnvironmentKind.venv, + NativePythonEnvironmentKind.venvUv, + ]; + for (const kind of kinds) { + const args = buildFindCliArgs(makeConfig(), kind); + const idx = args.indexOf('--kind'); + assert.ok(idx >= 0, `Expected --kind for ${kind}`); + assert.strictEqual(args[idx + 1], kind, `Expected kind value ${kind}`); + } + }); +}); + +// --------------------------------------------------------------------------- +// buildFindCliArgs — Uri[] options +// --------------------------------------------------------------------------- + +suite('buildFindCliArgs — Uri[] options', () => { + test('includes URI fsPaths as positional args', () => { + const uris = [Uri.file('/project/a'), Uri.file('/project/b')]; + const args = buildFindCliArgs(makeConfig(), uris); + // Uri.file on Windows will produce backslash paths; compare fsPath + assert.ok(args.includes(uris[0].fsPath), `Expected ${uris[0].fsPath} in args`); + assert.ok(args.includes(uris[1].fsPath), `Expected ${uris[1].fsPath} in args`); + }); + + test('adds --workspace flag when Uri[] provided (mirrors server mode workspace-only scan)', () => { + // In server mode, search_scope = Workspace when searchPaths is set, which skips all + // global discovery phases. --workspace mirrors that behaviour in the CLI fallback. + const uris = [Uri.file('/project/a')]; + const args = buildFindCliArgs(makeConfig(), uris); + assert.ok(args.includes('--workspace'), 'Should include --workspace flag for Uri[] paths'); + }); + + test('includes venvFolders as additional positional args', () => { + const uris = [Uri.file('/project/a')]; + const venvFolders = ['/home/user/.venvs', '/home/user/envs']; + const args = buildFindCliArgs(makeConfig(), uris, venvFolders); + assert.ok(args.includes('/home/user/.venvs'), 'Should include first venvFolder'); + assert.ok(args.includes('/home/user/envs'), 'Should include second venvFolder'); + }); + + test('does NOT include workspace dirs as positional args when Uri[] provided', () => { + const config = makeConfig({ workspaceDirectories: ['/workspace'] }); + const uris = [Uri.file('/project')]; + const args = buildFindCliArgs(config, uris); + assert.ok(!args.includes('/workspace'), 'Workspace dirs should be replaced by URI paths'); + }); + + test('does NOT add --kind flag when Uri[] provided', () => { + const uris = [Uri.file('/project')]; + const args = buildFindCliArgs(makeConfig(), uris); + assert.ok(!args.includes('--kind'), 'Should not include --kind'); + }); + + test('handles empty Uri[] with no venvFolders — only find --json --workspace', () => { + const args = buildFindCliArgs(makeConfig(), []); + assert.deepStrictEqual(args, ['find', '--json', '--workspace']); + }); +}); + +// --------------------------------------------------------------------------- +// buildFindCliArgs — configuration flags +// --------------------------------------------------------------------------- + +suite('buildFindCliArgs — configuration flags', () => { + test('adds --cache-directory when cacheDirectory is set', () => { + const config = makeConfig({ cacheDirectory: '/tmp/cache' }); + const args = buildFindCliArgs(config); + const idx = args.indexOf('--cache-directory'); + assert.ok(idx >= 0, 'Should include --cache-directory'); + assert.strictEqual(args[idx + 1], '/tmp/cache'); + }); + + test('omits --cache-directory when cacheDirectory is undefined', () => { + const args = buildFindCliArgs(makeConfig({ cacheDirectory: undefined })); + assert.ok(!args.includes('--cache-directory'), 'Should not include --cache-directory'); + }); + + test('adds --conda-executable when condaExecutable is set', () => { + const config = makeConfig({ condaExecutable: '/usr/bin/conda' }); + const args = buildFindCliArgs(config); + const idx = args.indexOf('--conda-executable'); + assert.ok(idx >= 0, 'Should include --conda-executable'); + assert.strictEqual(args[idx + 1], '/usr/bin/conda'); + }); + + test('omits --conda-executable when condaExecutable is undefined', () => { + const args = buildFindCliArgs(makeConfig()); + assert.ok(!args.includes('--conda-executable'), 'Should not include --conda-executable'); + }); + + test('adds --pipenv-executable when pipenvExecutable is set', () => { + const config = makeConfig({ pipenvExecutable: '/home/user/.local/bin/pipenv' }); + const args = buildFindCliArgs(config); + const idx = args.indexOf('--pipenv-executable'); + assert.ok(idx >= 0, 'Should include --pipenv-executable'); + assert.strictEqual(args[idx + 1], '/home/user/.local/bin/pipenv'); + }); + + test('adds --poetry-executable when poetryExecutable is set', () => { + const config = makeConfig({ poetryExecutable: '/home/user/.local/bin/poetry' }); + const args = buildFindCliArgs(config); + const idx = args.indexOf('--poetry-executable'); + assert.ok(idx >= 0, 'Should include --poetry-executable'); + assert.strictEqual(args[idx + 1], '/home/user/.local/bin/poetry'); + }); + + test('adds --environment-directories as comma-joined string', () => { + const config = makeConfig({ environmentDirectories: ['/home/.venvs', '/opt/envs'] }); + const args = buildFindCliArgs(config); + const idx = args.indexOf('--environment-directories'); + assert.ok(idx >= 0, 'Should include --environment-directories'); + assert.strictEqual(args[idx + 1], '/home/.venvs,/opt/envs', 'Dirs should be comma-joined'); + }); + + test('omits --environment-directories when array is empty', () => { + const args = buildFindCliArgs(makeConfig({ environmentDirectories: [] })); + assert.ok(!args.includes('--environment-directories'), 'Should not include --environment-directories'); + }); + + test('includes all config flags together', () => { + const config = makeConfig({ + workspaceDirectories: ['/workspace'], + environmentDirectories: ['/envs'], + condaExecutable: '/conda', + pipenvExecutable: '/pipenv', + poetryExecutable: '/poetry', + cacheDirectory: '/cache', + }); + const args = buildFindCliArgs(config); + assert.ok(args.includes('--cache-directory')); + assert.ok(args.includes('--conda-executable')); + assert.ok(args.includes('--pipenv-executable')); + assert.ok(args.includes('--poetry-executable')); + assert.ok(args.includes('--environment-directories')); + assert.ok(args.includes('/workspace'), 'Workspace dir should be positional'); + }); +}); + +// --------------------------------------------------------------------------- +// buildFindCliArgs — edge cases +// --------------------------------------------------------------------------- + +suite('buildFindCliArgs — edge cases', () => { + test('paths with spaces are passed as-is (not shell-quoted)', () => { + const config = makeConfig({ workspaceDirectories: ['/path with spaces/project'] }); + const args = buildFindCliArgs(config); + // The path should appear as-is without extra quoting — spawnProcess handles quoting + assert.ok(args.includes('/path with spaces/project')); + }); + + test('environmentDirectories with a single entry produces no comma', () => { + const config = makeConfig({ environmentDirectories: ['/only-one'] }); + const args = buildFindCliArgs(config); + const idx = args.indexOf('--environment-directories'); + assert.ok(idx >= 0); + assert.strictEqual(args[idx + 1], '/only-one'); + assert.ok(!args[idx + 1].includes(','), 'Single entry should not have comma'); + }); + + test('venvFolders are not added when options is a kind string', () => { + const venvFolders = ['/home/.venvs']; + const args = buildFindCliArgs(makeConfig(), NativePythonEnvironmentKind.conda, venvFolders); + // venvFolders are only positional args for Uri[], not for kind filters + assert.ok(!args.includes('/home/.venvs'), 'venvFolders should not be added for kind filter'); + }); + + test('venvFolders default to [] when not passed', () => { + const uris = [Uri.file('/project')]; + // Should not throw even without venvFolders parameter + const args = buildFindCliArgs(makeConfig(), uris); + assert.ok(args.includes(uris[0].fsPath)); + }); +}); + +// --------------------------------------------------------------------------- +// parseRefreshCliOutput — plan checklist items 2, 3, 4 +// --------------------------------------------------------------------------- + +suite('parseRefreshCliOutput — valid JSON', () => { + const manager: NativeEnvManagerInfo = { tool: 'Conda', executable: '/usr/bin/conda', version: '24.1.0' }; + const env: NativeEnvInfo = { + executable: '/usr/bin/python3', + kind: NativePythonEnvironmentKind.linuxGlobal, + version: '3.12.1', + prefix: '/usr', + }; + + test('returns managers and environments from well-formed output', () => { + const stdout = JSON.stringify({ managers: [manager], environments: [env] }); + const result = parseRefreshCliOutput(stdout); + assert.strictEqual(result.managers.length, 1); + assert.strictEqual(result.managers[0].tool, 'Conda'); + assert.strictEqual(result.environments.length, 1); + assert.strictEqual(result.environments[0].executable, '/usr/bin/python3'); + }); + + test('returns empty arrays when managers and environments are both absent', () => { + const result = parseRefreshCliOutput(JSON.stringify({})); + assert.deepStrictEqual(result.managers, []); + assert.deepStrictEqual(result.environments, []); + }); + + test('handles explicit empty managers array', () => { + const stdout = JSON.stringify({ managers: [], environments: [env] }); + const result = parseRefreshCliOutput(stdout); + assert.strictEqual(result.managers.length, 0); + assert.strictEqual(result.environments.length, 1); + }); + + test('handles explicit empty environments array', () => { + const stdout = JSON.stringify({ managers: [manager], environments: [] }); + const result = parseRefreshCliOutput(stdout); + assert.strictEqual(result.managers.length, 1); + assert.strictEqual(result.environments.length, 0); + }); + + test('handles both arrays empty', () => { + const result = parseRefreshCliOutput(JSON.stringify({ managers: [], environments: [] })); + assert.deepStrictEqual(result.managers, []); + assert.deepStrictEqual(result.environments, []); + }); + + test('returns multiple environments', () => { + const env2: NativeEnvInfo = { executable: '/usr/bin/python3.11', version: '3.11.0', prefix: '/usr' }; + const stdout = JSON.stringify({ managers: [], environments: [env, env2] }); + const result = parseRefreshCliOutput(stdout); + assert.strictEqual(result.environments.length, 2); + }); + + test('preserves all fields on environment objects', () => { + const richEnv: NativeEnvInfo = { + displayName: 'Python 3.12', + executable: '/usr/bin/python3', + kind: NativePythonEnvironmentKind.linuxGlobal, + version: '3.12.1', + prefix: '/usr', + arch: 'x64', + symlinks: ['/usr/bin/python3', '/usr/bin/python3.12'], + }; + const result = parseRefreshCliOutput(JSON.stringify({ managers: [], environments: [richEnv] })); + const parsed = result.environments[0]; + assert.strictEqual(parsed.displayName, 'Python 3.12'); + assert.deepStrictEqual(parsed.symlinks, ['/usr/bin/python3', '/usr/bin/python3.12']); + assert.strictEqual(parsed.arch, 'x64'); + }); + + test('environments with executable but missing version are returned as-is (incomplete env detection is caller responsibility)', () => { + const incompleteEnv: NativeEnvInfo = { executable: '/opt/myenv/bin/python' }; + const result = parseRefreshCliOutput(JSON.stringify({ managers: [], environments: [incompleteEnv] })); + assert.strictEqual(result.environments.length, 1); + assert.strictEqual(result.environments[0].executable, '/opt/myenv/bin/python'); + assert.strictEqual(result.environments[0].version, undefined); + assert.strictEqual(result.environments[0].prefix, undefined); + }); +}); + +suite('parseRefreshCliOutput — error cases', () => { + test('throws SyntaxError on malformed JSON', () => { + assert.throws(() => parseRefreshCliOutput('{not valid json'), SyntaxError); + }); + + test('throws SyntaxError on empty string', () => { + assert.throws(() => parseRefreshCliOutput(''), SyntaxError); + }); + + test('throws SyntaxError on JSON null (not an object)', () => { + assert.throws(() => parseRefreshCliOutput('null'), SyntaxError); + }); + + test('throws SyntaxError on JSON primitive', () => { + assert.throws(() => parseRefreshCliOutput('"just a string"'), SyntaxError); + }); +}); + +// --------------------------------------------------------------------------- +// parseResolveCliOutput — plan checklist items 5, 6, 7 +// --------------------------------------------------------------------------- + +suite('parseResolveCliOutput — valid JSON', () => { + const env: NativeEnvInfo = { + executable: '/home/user/project/.venv/bin/python', + kind: NativePythonEnvironmentKind.venv, + version: '3.12.0', + prefix: '/home/user/project/.venv', + }; + + test('returns NativeEnvInfo from valid environment JSON', () => { + const result = parseResolveCliOutput(JSON.stringify(env), env.executable!); + assert.strictEqual(result.executable, env.executable); + assert.strictEqual(result.version, '3.12.0'); + assert.strictEqual(result.prefix, '/home/user/project/.venv'); + }); + + test('preserves all fields', () => { + const richEnv: NativeEnvInfo = { + executable: '/home/user/.venv/bin/python', + kind: NativePythonEnvironmentKind.venv, + version: '3.11.5', + prefix: '/home/user/.venv', + arch: 'x64', + symlinks: ['/home/user/.venv/bin/python', '/home/user/.venv/bin/python3'], + name: 'myenv', + }; + const result = parseResolveCliOutput(JSON.stringify(richEnv), richEnv.executable!); + assert.strictEqual(result.name, 'myenv'); + assert.deepStrictEqual(result.symlinks, richEnv.symlinks); + assert.strictEqual(result.arch, 'x64'); + }); +}); + +suite('parseResolveCliOutput — null (environment not found)', () => { + test('throws Error when PET returns "null" (env not found)', () => { + assert.throws( + () => parseResolveCliOutput('null', '/usr/bin/python3'), + (err: Error) => { + assert.ok(err instanceof Error); + assert.ok(err.message.includes('/usr/bin/python3'), 'Error should mention the executable'); + return true; + }, + ); + }); + + test('error message identifies the executable', () => { + const exe = '/home/user/.venv/bin/python'; + let caught: Error | undefined; + try { + parseResolveCliOutput('null', exe); + } catch (ex) { + caught = ex as Error; + } + assert.ok(caught, 'Should have thrown'); + assert.ok(caught.message.includes(exe), `Error message "${caught.message}" should include ${exe}`); + }); +}); + +suite('parseResolveCliOutput — malformed stdout', () => { + test('throws SyntaxError on non-JSON output', () => { + assert.throws(() => parseResolveCliOutput('{bad json', '/usr/bin/python'), SyntaxError); + }); + + test('throws SyntaxError on empty string', () => { + assert.throws(() => parseResolveCliOutput('', '/usr/bin/python'), SyntaxError); + }); + + test('throws SyntaxError on partial JSON', () => { + assert.throws(() => parseResolveCliOutput('{"executable": "/usr/bin/py', '/usr/bin/python'), SyntaxError); + }); +}); From 23c44796f796eaddfe51587c05852435739176a1 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Mon, 13 Apr 2026 15:29:47 -0700 Subject: [PATCH 2/4] add logging, fix race condition --- src/managers/common/nativePythonFinder.ts | 94 ++++++++++++++----- .../nativePythonFinder.jsonCli.unit.test.ts | 37 ++++++-- 2 files changed, 101 insertions(+), 30 deletions(-) diff --git a/src/managers/common/nativePythonFinder.ts b/src/managers/common/nativePythonFinder.ts index 1d5b4148..79a72702 100644 --- a/src/managers/common/nativePythonFinder.ts +++ b/src/managers/common/nativePythonFinder.ts @@ -768,9 +768,15 @@ class NativePythonFinderImpl implements NativePythonFinder { /** * Returns true when all server restart attempts have been exhausted. * Used to decide whether to fall back to CLI mode. + * Does NOT return true while a restart is in progress — the server is not exhausted + * if it is still mid-restart (concurrent callers must not bypass to CLI prematurely). */ private isServerExhausted(): boolean { - return this.restartAttempts >= MAX_RESTART_ATTEMPTS && (this.startFailed || this.processExited); + return ( + !this.isRestarting && + this.restartAttempts >= MAX_RESTART_ATTEMPTS && + (this.startFailed || this.processExited) + ); } /** @@ -786,8 +792,16 @@ class NativePythonFinderImpl implements NativePythonFinder { return new Promise((resolve, reject) => { const proc = spawnProcess(this.toolPath, args, { stdio: 'pipe' }); let stdout = ''; + // Guard against settling the promise more than once. + // The timeout handler and the 'close'/'error' handlers can both fire + // (e.g. timeout fires → SIGTERM sent → close event fires shortly after). + let settled = false; const timer = setTimeout(() => { + if (settled) { + return; + } + settled = true; try { proc.kill('SIGTERM'); // Force kill after a short grace period if still running @@ -810,16 +824,29 @@ class NativePythonFinderImpl implements NativePythonFinder { this.outputChannel.debug(`[pet CLI] ${data.toString().trimEnd()}`); }); proc.on('close', (code) => { + if (settled) { + return; + } clearTimeout(timer); + settled = true; // If the process failed and produced no output, reject so caller gets a clear error if (code !== 0 && stdout.trim().length === 0) { reject(new Error(`PET CLI process exited with code ${code}`)); return; } + if (code !== 0) { + this.outputChannel.warn( + `[pet CLI] Process exited with code ${code} but produced output; using output`, + ); + } resolve(stdout); }); proc.on('error', (err) => { + if (settled) { + return; + } clearTimeout(timer); + settled = true; reject(err); }); }); @@ -874,25 +901,31 @@ class NativePythonFinderImpl implements NativePythonFinder { nativeInfo.push(manager); } + // Resolve incomplete environments in parallel, mirroring doRefreshAttempt's Promise.all pattern. + const resolvePromises: Promise[] = []; for (const env of parsed.environments ?? []) { if (env.executable && (!env.version || !env.prefix)) { // Environment has an executable but incomplete metadata — resolve individually - try { - const resolved = await this.resolveViaJsonCli(env.executable); - this.outputChannel.info(`[pet CLI] Resolved env: ${resolved.executable}`); - nativeInfo.push(resolved); - } catch { - // If resolve fails, still include the partial env so nothing is silently dropped - this.outputChannel.warn( - `[pet CLI] Could not resolve incomplete env, using partial data: ${env.executable}`, - ); - nativeInfo.push(env); - } + resolvePromises.push( + this.resolveViaJsonCli(env.executable) + .then((resolved) => { + this.outputChannel.info(`[pet CLI] Resolved env: ${resolved.executable}`); + nativeInfo.push(resolved); + }) + .catch(() => { + // If resolve fails, still include the partial env so nothing is silently dropped + this.outputChannel.warn( + `[pet CLI] Could not resolve incomplete env, using partial data: ${env.executable}`, + ); + nativeInfo.push(env); + }), + ); } else { this.outputChannel.info(`[pet CLI] Discovered env: ${env.executable ?? env.prefix}`); nativeInfo.push(env); } } + await Promise.all(resolvePromises); sendTelemetryEvent(EventNames.PET_JSON_CLI_FALLBACK, stopWatch.elapsedTime, { operation: 'refresh', @@ -926,6 +959,7 @@ class NativePythonFinderImpl implements NativePythonFinder { operation: 'resolve', result: 'error', }); + this.outputChannel.error('[pet] JSON CLI fallback resolve failed:', ex); throw ex; } @@ -1019,7 +1053,7 @@ export function parseRefreshCliOutput(stdout: string): { } { // May throw SyntaxError on malformed JSON — callers must handle const parsed = JSON.parse(stdout); - if (typeof parsed !== 'object' || parsed === null) { + if (typeof parsed !== 'object' || parsed === null || Array.isArray(parsed)) { throw new SyntaxError('PET find --json output is not a JSON object'); } return { @@ -1042,6 +1076,9 @@ export function parseResolveCliOutput(stdout: string, executable: string): Nativ if (parsed === null) { throw new Error(`PET could not identify environment for executable: ${executable}`); } + if (typeof parsed !== 'object' || Array.isArray(parsed)) { + throw new SyntaxError(`PET resolve --json output is not a JSON object for ${executable}`); + } return parsed; } @@ -1077,12 +1114,23 @@ export function buildFindCliArgs( // In server mode, `build_refresh_config` sets search_scope = Workspace, which causes // find_and_report_envs to skip all global discovery phases (locators, PATH, global venvs) // and only search the provided paths. Mirror that with --workspace. - args.push('--workspace'); - for (const uri of options) { - args.push(uri.fsPath); - } - for (const folder of venvFolders) { - args.push(folder); + // + // Edge case: if both options and venvFolders are empty, omit --workspace entirely. + // PET's CLI has no "search nothing" mode — with --workspace but no positional paths it + // falls back to CWD. Falling through to the workspace-dirs path is a better approximation + // of server-mode's empty-searchPaths behavior (which searches nothing meaningful) and + // avoids scanning an arbitrary directory. + const searchPaths = [...options.map((u) => u.fsPath), ...venvFolders]; + if (searchPaths.length > 0) { + args.push('--workspace'); + for (const p of searchPaths) { + args.push(p); + } + } else { + // No search paths at all: fall back to workspace dirs as positional args + for (const dir of config.workspaceDirectories) { + args.push(dir); + } } } } else { @@ -1105,9 +1153,11 @@ export function buildFindCliArgs( if (config.poetryExecutable) { args.push('--poetry-executable', config.poetryExecutable); } - if (config.environmentDirectories.length > 0) { - // PET accepts comma-separated dirs for --environment-directories - args.push('--environment-directories', config.environmentDirectories.join(',')); + // Pass each environment directory as a separate flag repetition. + // PET's --environment-directories uses value_delimiter=',' for env-var parsing, but + // repeating the flag on the CLI is the safe way to handle paths that contain commas. + for (const dir of config.environmentDirectories) { + args.push('--environment-directories', dir); } return args; diff --git a/src/test/managers/common/nativePythonFinder.jsonCli.unit.test.ts b/src/test/managers/common/nativePythonFinder.jsonCli.unit.test.ts index ceb468b9..3a8b71f6 100644 --- a/src/test/managers/common/nativePythonFinder.jsonCli.unit.test.ts +++ b/src/test/managers/common/nativePythonFinder.jsonCli.unit.test.ts @@ -140,9 +140,18 @@ suite('buildFindCliArgs — Uri[] options', () => { assert.ok(!args.includes('--kind'), 'Should not include --kind'); }); - test('handles empty Uri[] with no venvFolders — only find --json --workspace', () => { + test('handles empty Uri[] with no venvFolders — falls back to workspace dirs, omits --workspace', () => { + // PET's CLI with --workspace but no positional paths falls back to CWD, not an empty search. + // When both options and venvFolders are empty, omit --workspace and use workspaceDirs instead. + const config = makeConfig({ workspaceDirectories: ['/myworkspace'] }); + const args = buildFindCliArgs(config, []); + assert.ok(!args.includes('--workspace'), 'Should not include --workspace when no paths'); + assert.ok(args.includes('/myworkspace'), 'Should fall back to workspace dirs'); + }); + + test('handles empty Uri[] with no venvFolders and no workspaceDirs — only find --json', () => { const args = buildFindCliArgs(makeConfig(), []); - assert.deepStrictEqual(args, ['find', '--json', '--workspace']); + assert.deepStrictEqual(args, ['find', '--json']); }); }); @@ -193,12 +202,25 @@ suite('buildFindCliArgs — configuration flags', () => { assert.strictEqual(args[idx + 1], '/home/user/.local/bin/poetry'); }); - test('adds --environment-directories as comma-joined string', () => { + test('passes each environment directory as a separate flag (not comma-joined)', () => { const config = makeConfig({ environmentDirectories: ['/home/.venvs', '/opt/envs'] }); const args = buildFindCliArgs(config); - const idx = args.indexOf('--environment-directories'); - assert.ok(idx >= 0, 'Should include --environment-directories'); - assert.strictEqual(args[idx + 1], '/home/.venvs,/opt/envs', 'Dirs should be comma-joined'); + // Each dir must appear as a separate --environment-directories flag + // (comma-joining breaks paths that contain commas on POSIX/Windows) + const flagIndices = args.reduce( + (acc, a, i) => (a === '--environment-directories' ? [...acc, i] : acc), + [], + ); + assert.strictEqual(flagIndices.length, 2, 'Should have two --environment-directories flags'); + assert.strictEqual(args[flagIndices[0] + 1], '/home/.venvs'); + assert.strictEqual(args[flagIndices[1] + 1], '/opt/envs'); + }); + + test('paths with commas in environment-directories are passed safely (no splitting)', () => { + const config = makeConfig({ environmentDirectories: ['/my,path/envs', '/normal/envs'] }); + const args = buildFindCliArgs(config); + assert.ok(args.includes('/my,path/envs'), 'Comma-containing path should appear as-is'); + assert.ok(args.includes('/normal/envs')); }); test('omits --environment-directories when array is empty', () => { @@ -237,13 +259,12 @@ suite('buildFindCliArgs — edge cases', () => { assert.ok(args.includes('/path with spaces/project')); }); - test('environmentDirectories with a single entry produces no comma', () => { + test('environmentDirectories with a single entry passes exactly one flag', () => { const config = makeConfig({ environmentDirectories: ['/only-one'] }); const args = buildFindCliArgs(config); const idx = args.indexOf('--environment-directories'); assert.ok(idx >= 0); assert.strictEqual(args[idx + 1], '/only-one'); - assert.ok(!args[idx + 1].includes(','), 'Single entry should not have comma'); }); test('venvFolders are not added when options is a kind string', () => { From 27cbd627579a1e372b1cfd2f3e3c36f7d91941c8 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Wed, 15 Apr 2026 13:56:14 -0700 Subject: [PATCH 3/4] address comments --- src/managers/common/nativePythonFinder.ts | 44 +++++++++++++++-------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/src/managers/common/nativePythonFinder.ts b/src/managers/common/nativePythonFinder.ts index 79a72702..161520de 100644 --- a/src/managers/common/nativePythonFinder.ts +++ b/src/managers/common/nativePythonFinder.ts @@ -26,6 +26,8 @@ const RESOLVE_TIMEOUT_MS = 30_000; // 30 seconds for single resolve // CLI fallback timeout: generous budget since it's a full process spawn doing a full scan const CLI_FALLBACK_TIMEOUT_MS = 120_000; // 2 minutes +// Limit concurrent resolve subprocesses to avoid CPU/memory pressure on machines with many envs +const CLI_RESOLVE_CONCURRENCY = 4; // Restart/recovery constants const MAX_RESTART_ATTEMPTS = 3; @@ -885,13 +887,13 @@ class NativePythonFinderImpl implements NativePythonFinder { let parsed: { managers: NativeEnvManagerInfo[]; environments: NativeEnvInfo[] }; try { parsed = parseRefreshCliOutput(stdout); - } catch { + } catch (ex) { sendTelemetryEvent(EventNames.PET_JSON_CLI_FALLBACK, stopWatch.elapsedTime, { operation: 'refresh', result: 'error', }); - this.outputChannel.error('[pet] JSON CLI fallback: Failed to parse find output:', stdout.slice(0, 500)); - throw new Error('Failed to parse PET find --json output'); + this.outputChannel.error('[pet] JSON CLI fallback: Failed to parse find output:', stdout.slice(0, 500), ex); + throw new Error('Failed to parse PET find --json output', { cause: ex }); } const nativeInfo: NativeInfo[] = []; @@ -901,13 +903,28 @@ class NativePythonFinderImpl implements NativePythonFinder { nativeInfo.push(manager); } - // Resolve incomplete environments in parallel, mirroring doRefreshAttempt's Promise.all pattern. - const resolvePromises: Promise[] = []; + // Collect environments that need individual resolve calls. + // Incomplete environments have an executable but are missing version or prefix. + const toResolve: NativeEnvInfo[] = []; for (const env of parsed.environments ?? []) { if (env.executable && (!env.version || !env.prefix)) { - // Environment has an executable but incomplete metadata — resolve individually - resolvePromises.push( - this.resolveViaJsonCli(env.executable) + toResolve.push(env); + } else { + this.outputChannel.info(`[pet CLI] Discovered env: ${env.executable ?? env.prefix}`); + nativeInfo.push(env); + } + } + + // Resolve incomplete environments with bounded concurrency to avoid spawning too many + // subprocesses at once on machines with many incomplete environments. + // Each resolveViaJsonCli() spawns a new OS process, unlike server mode where all resolve + // calls share a single long-lived process — so unbounded parallelism would cause CPU/memory + // pressure. Process in batches of CLI_RESOLVE_CONCURRENCY. + for (let i = 0; i < toResolve.length; i += CLI_RESOLVE_CONCURRENCY) { + const batch = toResolve.slice(i, i + CLI_RESOLVE_CONCURRENCY); + await Promise.all( + batch.map((env) => + this.resolveViaJsonCli(env.executable!) .then((resolved) => { this.outputChannel.info(`[pet CLI] Resolved env: ${resolved.executable}`); nativeInfo.push(resolved); @@ -919,13 +936,9 @@ class NativePythonFinderImpl implements NativePythonFinder { ); nativeInfo.push(env); }), - ); - } else { - this.outputChannel.info(`[pet CLI] Discovered env: ${env.executable ?? env.prefix}`); - nativeInfo.push(env); - } + ), + ); } - await Promise.all(resolvePromises); sendTelemetryEvent(EventNames.PET_JSON_CLI_FALLBACK, stopWatch.elapsedTime, { operation: 'refresh', @@ -1090,7 +1103,8 @@ export function parseResolveCliOutput(stdout: string, executable: string): Nativ * @param options Optional refresh options: a kind filter string or an array of URIs to search. * @param venvFolders Additional virtual environment folder paths to include when searching * URI-based paths (needed because searchPaths may override environmentDirectories in PET). - * @returns The args array to pass to the PET binary (after 'find --json'). + * @returns The args array to pass directly to the PET binary, starting with `['find', '--json']` + * followed by the positional search paths and configuration flags. */ export function buildFindCliArgs( config: ConfigurationOptions, From eca318fed700c8538b03ad3312932212af2e1f75 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Wed, 15 Apr 2026 14:01:08 -0700 Subject: [PATCH 4/4] fix failure --- src/managers/common/nativePythonFinder.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/managers/common/nativePythonFinder.ts b/src/managers/common/nativePythonFinder.ts index 161520de..74664fa4 100644 --- a/src/managers/common/nativePythonFinder.ts +++ b/src/managers/common/nativePythonFinder.ts @@ -892,8 +892,12 @@ class NativePythonFinderImpl implements NativePythonFinder { operation: 'refresh', result: 'error', }); - this.outputChannel.error('[pet] JSON CLI fallback: Failed to parse find output:', stdout.slice(0, 500), ex); - throw new Error('Failed to parse PET find --json output', { cause: ex }); + this.outputChannel.error( + `[pet] JSON CLI fallback: Failed to parse find output (first 500 chars): ${stdout.slice(0, 500)}`, + ex, + ); + const cause = ex instanceof Error ? `: ${ex.message}` : ''; + throw new Error(`Failed to parse PET find --json output${cause}`); } const nativeInfo: NativeInfo[] = [];