From ad65e0733c3536fa5aa2af5f37663ca53052e1f4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Jun 2025 16:30:07 +0000 Subject: [PATCH 1/3] Initial plan From 66b9d21b0c2825c8b74e9b7811ef817de9f889a1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Jun 2025 16:42:10 +0000 Subject: [PATCH 2/3] Implement environment creation result handling and retry notifications for Quick Create Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> --- src/common/localize.ts | 8 ++ src/managers/builtin/venvManager.ts | 93 +++++++++++++++++++++- src/managers/builtin/venvUtils.ts | 117 ++++++++++++++++++++++++---- 3 files changed, 202 insertions(+), 16 deletions(-) diff --git a/src/common/localize.ts b/src/common/localize.ts index f1409c4d..3c3e362c 100644 --- a/src/common/localize.ts +++ b/src/common/localize.ts @@ -105,6 +105,14 @@ export namespace VenvManagerStrings { export const quickCreateDescription = l10n.t('Create a virtual environment in the workspace root'); export const customize = l10n.t('Custom'); export const customizeDescription = l10n.t('Choose python version, location, packages, name, etc.'); + + // New strings for retry functionality + export const packageInstallationFailedTitle = l10n.t('Package installation failed'); + export const packageInstallationFailedMessage = l10n.t('Environment was created successfully, but package installation failed. Would you like to select packages to install?'); + export const retryPackageInstallation = l10n.t('Select Packages'); + export const environmentCreationFailedTitle = l10n.t('Environment creation failed'); + export const environmentCreationFailedMessage = l10n.t('Failed to create virtual environment. Would you like to try again?'); + export const retryEnvironmentCreation = l10n.t('Retry'); } export namespace SysManagerStrings { diff --git a/src/managers/builtin/venvManager.ts b/src/managers/builtin/venvManager.ts index e655f6a9..072662e4 100644 --- a/src/managers/builtin/venvManager.ts +++ b/src/managers/builtin/venvManager.ts @@ -32,7 +32,7 @@ import { PYTHON_EXTENSION_ID } from '../../common/constants'; import { VenvManagerStrings } from '../../common/localize'; import { traceError } from '../../common/logging'; import { createDeferred, Deferred } from '../../common/utils/deferred'; -import { showErrorMessage, withProgress } from '../../common/window.apis'; +import { showErrorMessage, showInformationMessage, withProgress } from '../../common/window.apis'; import { findParentIfFile } from '../../features/envCommands'; import { NativePythonFinder } from '../common/nativePythonFinder'; import { getLatest, shortVersion, sortEnvironments } from '../common/utils'; @@ -51,6 +51,7 @@ import { setVenvForWorkspace, setVenvForWorkspaces, } from './venvUtils'; +import { getWorkspacePackagesToInstall } from './pipUtils'; export class VenvManager implements EnvironmentManager { private collection: PythonEnvironment[] = []; @@ -143,7 +144,7 @@ export class VenvManager implements EnvironmentManager { let environment: PythonEnvironment | undefined = undefined; if (options?.quickCreate) { if (this.globalEnv && this.globalEnv.version.startsWith('3.')) { - environment = await quickCreateVenv( + const result = await quickCreateVenv( this.nativeFinder, this.api, this.log, @@ -152,6 +153,28 @@ export class VenvManager implements EnvironmentManager { venvRoot, options?.additionalPackages, ); + + if (result.environmentCreated) { + environment = result.environment; + + // Handle package installation failure + if (!result.packagesInstalled && result.packageInstallationError) { + this.log.warn(`Quick Create: Package installation failed: ${result.packageInstallationError.message}`); + // Show notification with retry option for package installation + setImmediate(async () => { + await this.showPackageInstallationFailureNotification(environment!, venvRoot); + }); + } + } else { + // Environment creation failed completely + this.log.error(`Quick Create: Environment creation failed: ${result.environmentCreationError?.message}`); + // Show notification with retry option for entire environment creation + setImmediate(async () => { + await this.showEnvironmentCreationFailureNotification(scope, options); + }); + showErrorMessage(VenvManagerStrings.venvCreateFailed); + return; + } } else if (!this.globalEnv) { this.log.error('No base python found'); showErrorMessage(VenvManagerStrings.venvErrorNoBasePython); @@ -578,4 +601,70 @@ export class VenvManager implements EnvironmentManager { }); return projects; } + + /** + * Handles retry package installation flow after quick create. + */ + private async handlePackageInstallationRetry( + environment: PythonEnvironment, + venvRoot: Uri, + ): Promise { + try { + const project = this.api.getPythonProject(venvRoot); + const packages = await getWorkspacePackagesToInstall( + this.api, + { showSkipOption: true, install: [] }, + project ? [project] : undefined, + environment, + this.log, + ); + + if (packages && (packages.install.length > 0 || packages.uninstall.length > 0)) { + await this.api.managePackages(environment, { + upgrade: false, + install: packages.install, + uninstall: packages.uninstall, + }); + this.log.info('Package installation retry succeeded'); + } + } catch (error) { + this.log.error(`Package installation retry failed: ${error}`); + showErrorMessage(l10n.t('Package installation failed again: {0}', String(error))); + } + } + + /** + * Shows notification for package installation failure with retry option. + */ + private async showPackageInstallationFailureNotification( + environment: PythonEnvironment, + venvRoot: Uri, + ): Promise { + const result = await showInformationMessage( + VenvManagerStrings.packageInstallationFailedMessage, + VenvManagerStrings.retryPackageInstallation, + ); + + if (result === VenvManagerStrings.retryPackageInstallation) { + await this.handlePackageInstallationRetry(environment, venvRoot); + } + } + + /** + * Shows notification for environment creation failure with retry option. + */ + private async showEnvironmentCreationFailureNotification( + scope: CreateEnvironmentScope, + options: CreateEnvironmentOptions | undefined, + ): Promise { + const result = await showInformationMessage( + VenvManagerStrings.environmentCreationFailedMessage, + VenvManagerStrings.retryEnvironmentCreation, + ); + + if (result === VenvManagerStrings.retryEnvironmentCreation) { + // Retry the entire create flow + await this.create(scope, options); + } + } } diff --git a/src/managers/builtin/venvUtils.ts b/src/managers/builtin/venvUtils.ts index fe1ad231..59d3647c 100644 --- a/src/managers/builtin/venvUtils.ts +++ b/src/managers/builtin/venvUtils.ts @@ -33,6 +33,41 @@ import { resolveSystemPythonEnvironmentPath } from './utils'; export const VENV_WORKSPACE_KEY = `${ENVS_EXTENSION_ID}:venv:WORKSPACE_SELECTED`; export const VENV_GLOBAL_KEY = `${ENVS_EXTENSION_ID}:venv:GLOBAL_SELECTED`; +/** + * Result of environment creation operation. + */ +export interface CreateEnvironmentResult { + /** + * The created environment, if successful. + */ + environment?: PythonEnvironment; + + /** + * Whether environment creation succeeded. + */ + environmentCreated: boolean; + + /** + * Whether package installation succeeded (only relevant if environmentCreated is true). + */ + packagesInstalled: boolean; + + /** + * Error that occurred during environment creation. + */ + environmentCreationError?: Error; + + /** + * Error that occurred during package installation. + */ + packageInstallationError?: Error; + + /** + * Packages that were attempted to be installed. + */ + attemptedPackages?: PipPackages; +} + export async function clearVenvCache(): Promise { const keys = [VENV_WORKSPACE_KEY, VENV_GLOBAL_KEY]; const state = await getWorkspacePersistentState(); @@ -281,7 +316,7 @@ async function createWithProgress( venvRoot: Uri, envPath: string, packages?: PipPackages, -) { +): Promise { const pythonPath = os.platform() === 'win32' ? path.join(envPath, 'Scripts', 'python.exe') : path.join(envPath, 'bin', 'python'); @@ -290,7 +325,14 @@ async function createWithProgress( location: ProgressLocation.Notification, title: VenvManagerStrings.venvCreating, }, - async () => { + async (): Promise => { + let env: PythonEnvironment | undefined; + let environmentCreated = false; + let packagesInstalled = false; + let environmentCreationError: Error | undefined; + let packageInstallationError: Error | undefined; + + // Step 1: Create the virtual environment try { const useUv = await isUvInstalled(log); if (basePython.execInfo?.run.executable) { @@ -315,20 +357,48 @@ async function createWithProgress( } const resolved = await nativeFinder.resolve(pythonPath); - const env = api.createPythonEnvironmentItem(await getPythonInfo(resolved), manager); - if (packages && (packages.install.length > 0 || packages.uninstall.length > 0)) { + env = api.createPythonEnvironmentItem(await getPythonInfo(resolved), manager); + environmentCreated = true; + log.info(`Virtual environment created successfully: ${envPath}`); + } catch (e) { + environmentCreationError = e instanceof Error ? e : new Error(String(e)); + log.error(`Failed to create virtual environment: ${environmentCreationError.message}`); + return { + environmentCreated: false, + packagesInstalled: false, + environmentCreationError, + attemptedPackages: packages, + }; + } + + // Step 2: Install packages if environment was created successfully + if (env && packages && (packages.install.length > 0 || packages.uninstall.length > 0)) { + try { await api.managePackages(env, { upgrade: false, - install: packages?.install, - uninstall: packages?.uninstall ?? [], + install: packages.install, + uninstall: packages.uninstall ?? [], }); + packagesInstalled = true; + log.info('Packages installed successfully'); + } catch (e) { + packageInstallationError = e instanceof Error ? e : new Error(String(e)); + log.error(`Failed to install packages: ${packageInstallationError.message}`); + // Note: environment is still created, just packages failed } - return env; - } catch (e) { - log.error(`Failed to create virtual environment: ${e}`); - showErrorMessage(VenvManagerStrings.venvCreateFailed); - return; + } else { + // No packages to install, consider this successful + packagesInstalled = true; } + + return { + environment: env, + environmentCreated, + packagesInstalled, + environmentCreationError, + packageInstallationError, + attemptedPackages: packages, + }; }, ); } @@ -361,7 +431,7 @@ export async function quickCreateVenv( baseEnv: PythonEnvironment, venvRoot: Uri, additionalPackages?: string[], -): Promise { +): Promise { const project = api.getPythonProject(venvRoot); sendTelemetryEvent(EventNames.VENV_CREATION, undefined, { creationType: 'quick' }); @@ -408,7 +478,15 @@ export async function createPythonVenv( if (customize === undefined) { return; } else if (customize === false) { - return quickCreateVenv(nativeFinder, api, log, manager, sortedEnvs[0], venvRoot, options.additionalPackages); + const result = await quickCreateVenv(nativeFinder, api, log, manager, sortedEnvs[0], venvRoot, options.additionalPackages); + if (!result.environmentCreated) { + showErrorMessage(VenvManagerStrings.venvCreateFailed); + return; + } + if (!result.packagesInstalled && result.packageInstallationError) { + log.warn(`Package installation failed during quick create: ${result.packageInstallationError.message}`); + } + return result.environment; } else { sendTelemetryEvent(EventNames.VENV_CREATION, undefined, { creationType: 'custom' }); } @@ -450,10 +528,21 @@ export async function createPythonVenv( const allPackages = []; allPackages.push(...(packages?.install ?? []), ...(options.additionalPackages ?? [])); - return await createWithProgress(nativeFinder, api, log, manager, basePython, venvRoot, envPath, { + const result = await createWithProgress(nativeFinder, api, log, manager, basePython, venvRoot, envPath, { install: allPackages, uninstall: [], }); + + if (!result.environmentCreated) { + showErrorMessage(VenvManagerStrings.venvCreateFailed); + return; + } + + if (!result.packagesInstalled && result.packageInstallationError) { + log.warn(`Package installation failed: ${result.packageInstallationError.message}`); + } + + return result.environment; } export async function removeVenv(environment: PythonEnvironment, log: LogOutputChannel): Promise { From 1cb85c2146634469abea4b34840e80c8243eec84 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Jun 2025 16:45:40 +0000 Subject: [PATCH 3/3] Add unit tests for CreateEnvironmentResult and verify retry flow logic Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> --- .../managers/builtin/venvUtils.unit.test.ts | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 src/test/managers/builtin/venvUtils.unit.test.ts diff --git a/src/test/managers/builtin/venvUtils.unit.test.ts b/src/test/managers/builtin/venvUtils.unit.test.ts new file mode 100644 index 00000000..00043223 --- /dev/null +++ b/src/test/managers/builtin/venvUtils.unit.test.ts @@ -0,0 +1,66 @@ +import assert from 'assert'; +import { CreateEnvironmentResult } from '../../../managers/builtin/venvUtils'; + +suite('VenvUtils Tests', () => { + suite('CreateEnvironmentResult', () => { + test('should properly represent successful environment and package creation', () => { + const result: CreateEnvironmentResult = { + environment: undefined, // Mock environment would go here + environmentCreated: true, + packagesInstalled: true, + attemptedPackages: { install: ['pytest'], uninstall: [] }, + }; + + assert.strictEqual(result.environmentCreated, true); + assert.strictEqual(result.packagesInstalled, true); + assert.strictEqual(result.environmentCreationError, undefined); + assert.strictEqual(result.packageInstallationError, undefined); + assert.deepStrictEqual(result.attemptedPackages?.install, ['pytest']); + }); + + test('should properly represent environment creation success but package installation failure', () => { + const packageError = new Error('Failed to install packages'); + const result: CreateEnvironmentResult = { + environment: undefined, // Mock environment would go here + environmentCreated: true, + packagesInstalled: false, + packageInstallationError: packageError, + attemptedPackages: { install: ['conflicting-package'], uninstall: [] }, + }; + + assert.strictEqual(result.environmentCreated, true); + assert.strictEqual(result.packagesInstalled, false); + assert.strictEqual(result.environmentCreationError, undefined); + assert.strictEqual(result.packageInstallationError, packageError); + assert.deepStrictEqual(result.attemptedPackages?.install, ['conflicting-package']); + }); + + test('should properly represent complete environment creation failure', () => { + const envError = new Error('Failed to create environment'); + const result: CreateEnvironmentResult = { + environmentCreated: false, + packagesInstalled: false, + environmentCreationError: envError, + attemptedPackages: { install: ['some-package'], uninstall: [] }, + }; + + assert.strictEqual(result.environmentCreated, false); + assert.strictEqual(result.packagesInstalled, false); + assert.strictEqual(result.environment, undefined); + assert.strictEqual(result.environmentCreationError, envError); + assert.strictEqual(result.packageInstallationError, undefined); + }); + + test('should handle case with no packages to install', () => { + const result: CreateEnvironmentResult = { + environment: undefined, // Mock environment would go here + environmentCreated: true, + packagesInstalled: true, // No packages means "successful" + }; + + assert.strictEqual(result.environmentCreated, true); + assert.strictEqual(result.packagesInstalled, true); + assert.strictEqual(result.attemptedPackages, undefined); + }); + }); +}); \ No newline at end of file