From c3cb0231e33828ce8811b2960be2b67340c93812 Mon Sep 17 00:00:00 2001 From: Eduardo Villalpando Mello Date: Mon, 17 Aug 2026 19:37:13 -0400 Subject: [PATCH 1/4] test: cover registered package manager lifecycles (#1704) ## Summary Adds a package-manager-centric integration baseline that intentionally precedes and de-risks #1686, so the package-manager command refactor is exercised against behavior established on `main`. - drives one stateful install/list/direct-package/uninstall lifecycle per active profile - uses unique disposable projects and manager-owned disposable environments - exercises the live registered manager instances through a runtime-gated integration-test bridge - guards registry completeness so every registered package-manager ID has an active fixture or explicit deferral - covers normal Pip execution and Conda when their runtime prerequisites are available - records an uncached baseline instead of assuming a newly created environment is empty - restores workspace-scoped configuration from `inspect()` snapshots and performs guarded failure-safe cleanup - defers Poetry pending a Poetry-owned project/lockfile lifecycle - defers uv-backed Pip because changing the machine-scoped selection reliably within one extension host was not stable on `main`, while available-version lookup would also introduce `uv tool run pip` network seeding - pins the disposable integration-test user profile to normal Pip execution ## Validation - `npm run compile` - `npm run compile-tests` - `npm run lint` - `npm run unittest` - targeted `packageManagement.integration.test.js`: 3 passing, 2 prerequisite skips locally - Pip skipped because quick create selected Python 3.15.0 alpha, whose bundled Pip metadata is incomplete - Conda skipped because Conda is not installed - reviewer specialist: clean, no Critical or Important findings The active Pip and Conda fixtures require package-index/network access when their runtime prerequisites are present. Fixes #1701 --------- Copilot-Session: 6b2fe9b5-38ea-442f-b07a-b6c71134d480 Copilot-Session: 3fd1a810-6840-4ac9-ac33-c8a9fda4bfc4 --- .../testing-workflow.instructions.md | 1 + .github/workflows/pr-check.yml | 8 + .github/workflows/push-check.yml | 8 + api/CHANGELOG.md | 7 + api/package-lock.json | 4 +- api/package.json | 2 +- examples/sample1/src/api.ts | 111 ++++--- src/api.ts | 112 +++++--- src/extension.ts | 15 + src/features/pythonApi.ts | 9 +- src/internal.api.ts | 11 +- src/managers/builtin/pipPackageManager.ts | 73 +++-- src/managers/builtin/utils.ts | 6 +- src/managers/builtin/venvManager.ts | 15 +- src/managers/builtin/venvUtils.ts | 36 ++- src/managers/common/packageChanges.ts | 15 +- src/managers/conda/condaPackageManager.ts | 13 +- src/managers/poetry/poetryPackageManager.ts | 18 +- .../packageManagement.integration.test.ts | 8 +- .../packageManager.integration.test.ts | 271 ++++++++++++++++++ .../builtin/pipPackageManager.unit.test.ts | 20 ++ .../builtin/pipPackageRefresh.unit.test.ts | 53 ++++ .../managers/builtin/pipVersions.unit.test.ts | 35 ++- .../venvManager.createRemove.unit.test.ts | 22 +- .../builtin/venvUtils.removeVenv.unit.test.ts | 31 ++ .../common/packageChanges.unit.test.ts | 29 ++ .../conda/condaPackageManager.unit.test.ts | 41 +++ 27 files changed, 813 insertions(+), 161 deletions(-) create mode 100644 src/test/integration/packageManager.integration.test.ts create mode 100644 src/test/managers/builtin/pipPackageRefresh.unit.test.ts create mode 100644 src/test/managers/conda/condaPackageManager.unit.test.ts diff --git a/.github/instructions/testing-workflow.instructions.md b/.github/instructions/testing-workflow.instructions.md index b374c38d1..68958773b 100644 --- a/.github/instructions/testing-workflow.instructions.md +++ b/.github/instructions/testing-workflow.instructions.md @@ -606,3 +606,4 @@ envConfig.inspect - **Never skip tests to hide infrastructure problems**: If tests require native binaries (like `pet`), the CI workflow must build/download them. Skipping tests when infrastructure is missing gives false confidence. Build from source (like vscode-python does) rather than skipping. Tests should fail clearly when something is wrong (2) - **No retries for masking flakiness**: Mocha `retries` should not be used to mask test flakiness. If a test is flaky, fix the root cause. Retries hide real issues and slow down CI (1) - **pet binary is required for environment manager registration**: The smoke/E2E/integration tests require the `pet` binary from `microsoft/python-environment-tools` to be built and placed in `python-env-tools/bin/`. Without it, `waitForApiReady()` will timeout because managers never register. CI must build pet from source using `cargo build --release --package pet` (2) +- **Check exact project registration with `getPythonProjects()`**: `getPythonProject(uri)` can return a containing parent project, so it cannot prove that a nested project was registered or unregistered (1) diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml index 1298ffa0b..9297f7df6 100644 --- a/.github/workflows/pr-check.yml +++ b/.github/workflows/pr-check.yml @@ -335,6 +335,14 @@ jobs: if: runner.os != 'Linux' run: npm run integration-test + - name: Run Package Manager Network Integration Tests + if: runner.os == 'Linux' && matrix.python-version == '3.12' + uses: GabrielBB/xvfb-action@86d97bde4a65fe9b290c0b3fb92c2c4ed0e5302d # v1.6 + env: + VSC_PYTHON_PACKAGE_NETWORK_TEST: '1' + with: + run: npm run integration-test -- --grep "Package Manager" + integration-tests-multiroot: name: Integration Tests (Multi-Root) runs-on: ${{ matrix.os }} diff --git a/.github/workflows/push-check.yml b/.github/workflows/push-check.yml index 96867be26..23db9b117 100644 --- a/.github/workflows/push-check.yml +++ b/.github/workflows/push-check.yml @@ -335,3 +335,11 @@ jobs: - name: Run Integration Tests (non-Linux) if: runner.os != 'Linux' run: npm run integration-test + + - name: Run Package Manager Network Integration Tests + if: runner.os == 'Linux' && matrix.python-version == '3.12' + uses: GabrielBB/xvfb-action@86d97bde4a65fe9b290c0b3fb92c2c4ed0e5302d # v1.6 + env: + VSC_PYTHON_PACKAGE_NETWORK_TEST: '1' + with: + run: npm run integration-test -- --grep "Package Manager" diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md index 082eac300..616edca22 100644 --- a/api/CHANGELOG.md +++ b/api/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to the `@vscode/python-environments` API package are documen The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.2.0] + +### Added + +- Added `PackageManagementInteractionOptions` with an optional `runHeadless?: boolean` property, mixed into `PackageManagementOptions`. When `true`, package management operations run without any user prompts or interaction — steps that would normally require input, such as selecting packages to install when none are specified, are skipped instead of prompting — for automated or headless scenarios such as integration tests. +- Added `RemoveEnvironmentOptions` with an optional `runHeadless?: boolean` property to remove environments without a confirmation prompt in automated or headless scenarios. + ## [1.1.0] ### Added diff --git a/api/package-lock.json b/api/package-lock.json index 8363de4f9..7745eab9a 100644 --- a/api/package-lock.json +++ b/api/package-lock.json @@ -1,12 +1,12 @@ { "name": "@vscode/python-environments", - "version": "1.1.0", + "version": "1.2.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@vscode/python-environments", - "version": "1.1.0", + "version": "1.2.0", "license": "MIT", "dependencies": { "@renovatebot/pep440": "^3.1.0" diff --git a/api/package.json b/api/package.json index 7f68cf0b2..6b1c6e70e 100644 --- a/api/package.json +++ b/api/package.json @@ -1,7 +1,7 @@ { "name": "@vscode/python-environments", "description": "An API facade for the Python Environments extension in VS Code", - "version": "1.1.0", + "version": "1.2.0", "author": { "name": "Microsoft Corporation" }, diff --git a/examples/sample1/src/api.ts b/examples/sample1/src/api.ts index c45ae1cbd..00512e001 100644 --- a/examples/sample1/src/api.ts +++ b/examples/sample1/src/api.ts @@ -329,6 +329,17 @@ export interface QuickCreateConfig { readonly detail?: string; } +/** + * Options controlling environment removal. + */ +export interface RemoveEnvironmentOptions { + /** + * When `true`, removes the environment without prompting for confirmation. + * Intended for automated or headless scenarios. Defaults to `false`. + */ + runHeadless?: boolean; +} + /** * Interface representing an environment manager. */ @@ -392,7 +403,7 @@ export interface EnvironmentManager { * @param environment - The Python environment to remove. * @returns A promise that resolves when the environment is removed. */ - remove?(environment: PythonEnvironment): Promise; + remove?(environment: PythonEnvironment, options?: RemoveEnvironmentOptions): Promise; /** * Refreshes the list of Python environments within the specified scope. @@ -739,49 +750,62 @@ export interface GetPackagesOptions { } /** - * Options for package management. + * Options controlling user interaction during package management operations. */ -export type PackageManagementOptions = - | { - /** - * Upgrade the packages if it is already installed. - */ - upgrade?: boolean; - - /** - * Show option to skip package installation - */ - showSkipOption?: boolean; - /** - * The list of packages to install. - */ - install: string[]; - - /** - * The list of packages to uninstall. - */ - uninstall?: string[]; - } - | { - /** - * Upgrade the packages if it is already installed. - */ - upgrade?: boolean; - - /** - * Show option to skip package installation - */ - showSkipOption?: boolean; - /** - * The list of packages to install. - */ - install?: string[]; +export interface PackageManagementInteractionOptions { + /** + * When `true`, the package management operation runs without any user prompts or + * interaction and relies solely on the packages provided in the options. Any step + * that would normally require user input — such as selecting packages to install + * when none are specified — is skipped instead of prompting the user. Intended for + * automated or headless scenarios such as integration tests. Defaults to `false`. + */ + runHeadless?: boolean; +} - /** - * The list of packages to uninstall. - */ - uninstall: string[]; - }; +export type PackageManagementOptions = PackageManagementInteractionOptions & + ( + | { + /** + * Upgrade the packages if it is already installed. + */ + upgrade?: boolean; + + /** + * Show option to skip package installation or uninstallation. + */ + showSkipOption?: boolean; + /** + * The list of packages to install. + */ + install: string[]; + + /** + * The list of packages to uninstall. + */ + uninstall?: string[]; + } + | { + /** + * Upgrade the packages if it is already installed. + */ + upgrade?: boolean; + + /** + * Show option to skip package installation or uninstallation. + */ + showSkipOption?: boolean; + /** + * The list of packages to install. + */ + install?: string[]; + + /** + * The list of packages to uninstall. + */ + uninstall: string[]; + } + ); /** * Options for creating a Python environment. @@ -881,9 +905,10 @@ export interface PythonEnvironmentManagementApi { * Remove a Python environment. * * @param environment The Python environment to remove. + * @param options Optional parameters controlling environment removal. * @returns A promise that resolves when the environment has been removed. */ - removeEnvironment(environment: PythonEnvironment): Promise; + removeEnvironment(environment: PythonEnvironment, options?: RemoveEnvironmentOptions): Promise; } export interface PythonEnvironmentsApi { diff --git a/src/api.ts b/src/api.ts index 5d63a3aef..2779d27b0 100644 --- a/src/api.ts +++ b/src/api.ts @@ -345,6 +345,17 @@ export interface QuickCreateConfig { readonly detail?: string; } +/** + * Options controlling environment removal. + */ +export interface RemoveEnvironmentOptions { + /** + * When `true`, removes the environment without prompting for confirmation. + * Intended for automated or headless scenarios. Defaults to `false`. + */ + runHeadless?: boolean; +} + /** * Interface representing an environment manager. * @@ -425,7 +436,7 @@ export interface EnvironmentManager { * Invoked to delete the given environment. Typical triggers include an explicit user * action (such as a "Delete Environment" command) and programmatic removal via the API. */ - remove?(environment: PythonEnvironment): Promise; + remove?(environment: PythonEnvironment, options?: RemoveEnvironmentOptions): Promise; /** * Refreshes the list of Python environments within the specified scope. @@ -872,47 +883,63 @@ export interface GetPackagesOptions { skipCache?: boolean; } -export type PackageManagementOptions = - | { - /** - * Upgrade the packages if they are already installed. - */ - upgrade?: boolean; - - /** - * Show option to skip package installation or uninstallation. - */ - showSkipOption?: boolean; - /** - * The list of packages to install. - */ - install: string[]; - - /** - * The list of packages to uninstall. - */ - uninstall?: string[]; - } - | { - /** - * Upgrade the packages if they are already installed. - */ - upgrade?: boolean; - - /** - * Show option to skip package installation or uninstallation. - */ - showSkipOption?: boolean; - /** - * The list of packages to install. - */ - install?: string[]; +/** + * Options controlling user interaction during package management operations. + */ +export interface PackageManagementInteractionOptions { + /** + * When `true`, the package management operation runs without any user prompts or + * interaction and relies solely on the packages provided in the options. Any step + * that would normally require user input — such as selecting packages to install + * when none are specified — is skipped instead of prompting the user. Intended for + * automated or headless scenarios such as integration tests. Defaults to `false`. + */ + runHeadless?: boolean; +} - /** - * The list of packages to uninstall. - */ - uninstall: string[]; - }; +export type PackageManagementOptions = PackageManagementInteractionOptions & + ( + | { + /** + * Upgrade the packages if they are already installed. + */ + upgrade?: boolean; + + /** + * Show option to skip package installation or uninstallation. + */ + showSkipOption?: boolean; + /** + * The list of packages to install. + */ + install: string[]; + + /** + * The list of packages to uninstall. + */ + uninstall?: string[]; + } + | { + /** + * Upgrade the packages if they are already installed. + */ + upgrade?: boolean; + + /** + * Show option to skip package installation or uninstallation. + */ + showSkipOption?: boolean; + /** + * The list of packages to install. + */ + install?: string[]; + + /** + * The list of packages to uninstall. + */ + uninstall: string[]; + } + ); /** * Options for creating a Python environment. @@ -1011,9 +1038,10 @@ export interface PythonEnvironmentManagementApi { * Remove a Python environment. * * @param environment The Python environment to remove. + * @param options Optional parameters controlling environment removal. * @returns A promise that resolves when the environment has been removed. */ - removeEnvironment(environment: PythonEnvironment): Promise; + removeEnvironment(environment: PythonEnvironment, options?: RemoveEnvironmentOptions): Promise; } export interface PythonEnvironmentsApi { diff --git a/src/extension.ts b/src/extension.ts index 1d2d98aee..46f89009b 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -258,6 +258,21 @@ export async function activate(context: ExtensionContext): Promise + envManagers.packageManagers.map((manager) => manager.id), + ), + commands.registerCommand( + 'python-envs.test.getDirectPackageNames', + async (environment: PythonEnvironment) => { + const manager = envManagers.getPackageManager(environment); + const names = await manager?.getDirectPackageNames?.(environment); + return names ? Array.from(names) : undefined; + }, + ), + ] + : []), commands.registerCommand('python-envs.searchSettings', async () => { await openSearchSettings(); }), diff --git a/src/features/pythonApi.ts b/src/features/pythonApi.ts index 9c494b9eb..e93ed0cdb 100644 --- a/src/features/pythonApi.ts +++ b/src/features/pythonApi.ts @@ -29,6 +29,7 @@ import { PythonTerminalCreateOptions, PythonTerminalExecutionOptions, RefreshEnvironmentsScope, + RemoveEnvironmentOptions, ResolveEnvironmentContext, SetEnvironmentScope, } from '../api'; @@ -107,9 +108,7 @@ export class PythonEnvironmentApiImpl implements PythonEnvironmentApi { this.previousProjects = current; if (added.length > 0 || removed.length > 0) { - traceInfo( - `Python API: Projects changed. Added: ${added.length}, Removed: ${removed.length}`, - ); + traceInfo(`Python API: Projects changed. Added: ${added.length}, Removed: ${removed.length}`); this._onDidChangePythonProjects.fire({ added, removed }); } }), @@ -197,13 +196,13 @@ export class PythonEnvironmentApiImpl implements PythonEnvironmentApi { return result; } } - async removeEnvironment(environment: PythonEnvironment): Promise { + async removeEnvironment(environment: PythonEnvironment, options?: RemoveEnvironmentOptions): Promise { await waitForEnvManagerId([environment.envId.managerId]); const manager = this.envManagers.getEnvironmentManager(environment); if (!manager) { return Promise.reject(new Error('No environment manager found')); } - return manager.remove(environment); + return manager.remove(environment, options); } async refreshEnvironments(scope: RefreshEnvironmentsScope): Promise { const currentScope = checkUri(scope) as RefreshEnvironmentsScope; diff --git a/src/internal.api.ts b/src/internal.api.ts index 9b09d5cf8..6d41cb5c3 100644 --- a/src/internal.api.ts +++ b/src/internal.api.ts @@ -26,6 +26,7 @@ import { PythonProjectCreator, QuickCreateConfig, RefreshEnvironmentsScope, + RemoveEnvironmentOptions, ResolveEnvironmentContext, SetEnvironmentScope, } from './api'; @@ -208,9 +209,9 @@ export class InternalEnvironmentManager implements EnvironmentManager { return this.manager.remove !== undefined; } - remove(scope: PythonEnvironment): Promise { + remove(scope: PythonEnvironment, options?: RemoveEnvironmentOptions): Promise { return this.manager.remove - ? this.manager.remove(scope) + ? this.manager.remove(scope, options) : Promise.reject(new RemoveEnvironmentNotSupported(`Remove Environment not supported by: ${this.id}`)); } @@ -405,6 +406,12 @@ export class InternalPackageManager implements PackageManager { : Promise.resolve(undefined); } + getDirectPackageNames(environment: PythonEnvironment): Promise | undefined> { + return this.manager.getDirectPackageNames + ? this.manager.getDirectPackageNames(environment) + : Promise.resolve(undefined); + } + formatInstallSpec(packageName: string, version: string): string { return this.manager.formatInstallSpec ? this.manager.formatInstallSpec(packageName, version) diff --git a/src/managers/builtin/pipPackageManager.ts b/src/managers/builtin/pipPackageManager.ts index bd3bbb761..836244bb7 100644 --- a/src/managers/builtin/pipPackageManager.ts +++ b/src/managers/builtin/pipPackageManager.ts @@ -55,6 +55,10 @@ export class PipPackageManager implements PackageManager, Disposable { let toUninstall: string[] = [...(options.uninstall ?? [])]; if (toInstall.length === 0 && toUninstall.length === 0) { + if (options.runHeadless) { + // Headless mode: skip the interactive package picker. + return; + } const projects = this.venv.getProjectsByEnvironment(environment); const result = await getWorkspacePackagesToInstall(this.api, options, projects, environment, this.log); if (result) { @@ -86,18 +90,21 @@ export class PipPackageManager implements PackageManager, Disposable { (changes) => { this._onDidChangePackages.fire({ environment, manager: this, changes }); }, + () => this.fetchPackages(environment, !manageOptions.runHeadless), ); } catch (e) { if (e instanceof CancellationError) { throw e; } this.log.error('Error managing packages', e); - setImmediate(async () => { - const result = await window.showErrorMessage('Error managing packages', 'View Output'); - if (result === 'View Output') { - this.log.show(); - } - }); + if (!manageOptions.runHeadless) { + setImmediate(async () => { + const result = await window.showErrorMessage('Error managing packages', 'View Output'); + if (result === 'View Output') { + this.log.show(); + } + }); + } throw e; } }, @@ -119,25 +126,31 @@ export class PipPackageManager implements PackageManager, Disposable { this._onDidChangePackages.fire({ environment, manager: this, changes }); }, ); - this.packages.set(environment.envId.id, packages ?? []); + if (packages !== undefined) { + this.packages.set(environment.envId.id, packages); + } }, ); } async getPackages(environment: PythonEnvironment, options?: GetPackagesOptions): Promise { if (options?.skipCache || !this.packages.has(environment.envId.id)) { - const data = await refreshPipPackages(environment, this.log); - if (data === undefined) { - return this.packages.get(environment.envId.id); - } - - const packages = data.map((pkg) => this.api.createPackageItem(pkg, environment, this)); - this.packages.set(environment.envId.id, packages); - return packages; + return this.fetchPackages(environment); } return this.packages.get(environment.envId.id); } + private async fetchPackages(environment: PythonEnvironment, showErrors = true): Promise { + const data = await refreshPipPackages(environment, this.log, { showErrors }); + if (data === undefined) { + return this.packages.get(environment.envId.id); + } + + const packages = data.map((pkg) => this.api.createPackageItem(pkg, environment, this)); + this.packages.set(environment.envId.id, packages); + return packages; + } + async getVersion(environment: PythonEnvironment): Promise { try { const useUv = await shouldUseUv(this.log, environment.environmentPath.fsPath); @@ -186,9 +199,9 @@ export class PipPackageManager implements PackageManager, Disposable { return parsePipIndexVersionsJson(output); } - // pip >= 21.2.0 - use `pip index versions --json` to get available versions in a machine readable format. + // pip >= 25.1 - use `pip index versions --json` to get available versions in a machine readable format. const pipVersion = await this.getVersion(environment); - if (pipVersion && compare(pipVersion.public, '21.2.0') >= 0) { + if (pipVersion && compare(pipVersion.public, '25.1') >= 0) { const output = await runPython( python, ['-m', 'pip', 'index', 'versions', packageName, '--json', '--python-version', baseVersion], @@ -198,7 +211,17 @@ export class PipPackageManager implements PackageManager, Disposable { return parsePipIndexVersionsJson(output); } - // pip <= 20.3.4 - version picking is undefined; no reliable machine-readable API exists. + if (pipVersion && compare(pipVersion.public, '21.2') >= 0) { + const output = await runPython( + python, + ['-m', 'pip', 'index', 'versions', packageName, '--python-version', baseVersion], + undefined, + this.log, + ); + return parsePipIndexVersionsText(output); + } + + // pip < 21.2 - version picking is undefined; `pip index versions` is unavailable. } catch { return undefined; } @@ -245,3 +268,17 @@ export function parsePipIndexVersionsJson(output: string): Pep440Version[] | und return undefined; } } + +/** Parses the legacy text output from `pip index versions `. */ +export function parsePipIndexVersionsText(output: string): Pep440Version[] | undefined { + const match = output.match(/^Available versions:\s*(.+)$/im); + if (!match) { + return undefined; + } + const versions = match[1] + .split(',') + .map((version) => parse(version.trim())) + .filter((version): version is Pep440Version => version !== null) + .sort((a, b) => rcompare(a.public, b.public)); + return versions.length > 0 ? versions : undefined; +} diff --git a/src/managers/builtin/utils.ts b/src/managers/builtin/utils.ts index dc44fe759..f6ff2903a 100644 --- a/src/managers/builtin/utils.ts +++ b/src/managers/builtin/utils.ts @@ -218,7 +218,7 @@ async function execPipList(environment: PythonEnvironment, log?: LogOutputChanne export async function refreshPipPackages( environment: PythonEnvironment, log?: LogOutputChannel, - options?: { showProgress: boolean }, + options?: { showProgress?: boolean; showErrors?: boolean }, ): Promise { let data: string; try { @@ -238,7 +238,9 @@ export async function refreshPipPackages( return parsePipListJson(data, log); } catch (e) { log?.error('Error refreshing packages', e); - showErrorMessageWithLogs(SysManagerStrings.packageRefreshError, log); + if (options?.showErrors !== false) { + showErrorMessageWithLogs(SysManagerStrings.packageRefreshError, log); + } return undefined; } } diff --git a/src/managers/builtin/venvManager.ts b/src/managers/builtin/venvManager.ts index 7af0f450a..6dcda4df8 100644 --- a/src/managers/builtin/venvManager.ts +++ b/src/managers/builtin/venvManager.ts @@ -1,14 +1,6 @@ import * as fs from 'fs/promises'; import * as path from 'path'; -import { - EventEmitter, - l10n, - LogOutputChannel, - MarkdownString, - ProgressLocation, - ThemeIcon, - Uri, -} from 'vscode'; +import { EventEmitter, l10n, LogOutputChannel, MarkdownString, ProgressLocation, ThemeIcon, Uri } from 'vscode'; import { CreateEnvironmentOptions, CreateEnvironmentScope, @@ -24,6 +16,7 @@ import { PythonProject, QuickCreateConfig, RefreshEnvironmentsScope, + RemoveEnvironmentOptions, ResolveEnvironmentContext, SetEnvironmentScope, } from '../../api'; @@ -265,11 +258,11 @@ export class VenvManager implements EnvironmentManager { /** * Removes the specified Python environment, updates internal collections, and fires change events as needed. */ - async remove(environment: PythonEnvironment): Promise { + async remove(environment: PythonEnvironment, options?: RemoveEnvironmentOptions): Promise { try { this.skipWatcherRefresh = true; - const isRemoved = await removeVenv(environment, this.log); + const isRemoved = await removeVenv(environment, this.log, options); if (!isRemoved) { return; } diff --git a/src/managers/builtin/venvUtils.ts b/src/managers/builtin/venvUtils.ts index c06146999..2962235e1 100644 --- a/src/managers/builtin/venvUtils.ts +++ b/src/managers/builtin/venvUtils.ts @@ -11,7 +11,13 @@ import { ThemeIcon, Uri, } from 'vscode'; -import { EnvironmentManager, PythonEnvironment, PythonEnvironmentApi, PythonEnvironmentInfo } from '../../api'; +import { + EnvironmentManager, + PythonEnvironment, + PythonEnvironmentApi, + PythonEnvironmentInfo, + RemoveEnvironmentOptions, +} from '../../api'; import { ENVS_EXTENSION_ID } from '../../common/constants'; import { Common, VenvManagerStrings } from '../../common/localize'; import { traceInfo, traceVerbose } from '../../common/logging'; @@ -553,7 +559,11 @@ async function validateVenvRemovalPath(envPath: string, log: LogOutputChannel): return undefined; } -export async function removeVenv(environment: PythonEnvironment, log: LogOutputChannel): Promise { +export async function removeVenv( + environment: PythonEnvironment, + log: LogOutputChannel, + options?: RemoveEnvironmentOptions, +): Promise { const pythonPath = os.platform() === 'win32' ? 'python.exe' : 'python'; const envFsPath = path.normalize(environment.environmentPath.fsPath); @@ -568,15 +578,19 @@ export async function removeVenv(environment: PythonEnvironment, log: LogOutputC // Normalize path for UI display - ensure forward slashes on Windows const displayPath = normalizePath(envPath); - const confirm = await showWarningMessage( - l10n.t('Are you sure you want to remove {0}?', displayPath), - { - modal: true, - }, - { title: Common.yes }, - { title: Common.no, isCloseAffordance: true }, - ); - if (confirm?.title === Common.yes) { + const confirmed = + options?.runHeadless === true || + ( + await showWarningMessage( + l10n.t('Are you sure you want to remove {0}?', displayPath), + { + modal: true, + }, + { title: Common.yes }, + { title: Common.no, isCloseAffordance: true }, + ) + )?.title === Common.yes; + if (confirmed) { const result = await withProgress( { location: ProgressLocation.Notification, diff --git a/src/managers/common/packageChanges.ts b/src/managers/common/packageChanges.ts index 3e16ae361..6c484fccd 100644 --- a/src/managers/common/packageChanges.ts +++ b/src/managers/common/packageChanges.ts @@ -9,6 +9,8 @@ import { normalizePackageName } from '../builtin/utils'; */ export type PackageChangesCallback = (changes: { kind: PackageChangeKind; pkg: Package }[]) => void; +type PackageFetcher = () => Promise; + /** * Computes the list of package changes between a before and after snapshot. * @param before - The previous list of packages. @@ -41,19 +43,30 @@ export function getPackageChanges(before: Package[], after: Package[]): { kind: * This function calls {@link PackageManager.getPackages} with `skipCache` to fetch * the latest snapshot. The caller should pass the previously cached packages * so changes can be computed against the pre-refresh state. + * + * @param packageManager The package manager whose packages changed. + * @param environment The environment whose packages should be refreshed. + * @param before The package snapshot from before the operation. + * @param onChanges Callback invoked when package changes are detected. + * @param fetchPackages Optional internal fetcher for operation-specific refresh behavior. */ export async function updatePackagesAndNotify( packageManager: PackageManager, environment: PythonEnvironment, before: Package[] | undefined, onChanges: PackageChangesCallback, + fetchPackages?: PackageFetcher, ): Promise { const [after, afterDirectDependenciesNames] = await Promise.all([ - packageManager.getPackages(environment, { skipCache: true }).then((pkgs) => pkgs ?? []), + fetchPackages?.() ?? packageManager.getPackages(environment, { skipCache: true }), // Handle transitive dependencies (best-effort, don't break package refresh on failure) packageManager.getDirectPackageNames?.(environment).catch(() => undefined), ]); + if (after === undefined) { + return undefined; + } + // Enrich packages with transitive dependency info (best-effort, creates new objects to respect readonly) const enriched = afterDirectDependenciesNames && afterDirectDependenciesNames.size > 0 ? after.map((pkg) => ({ diff --git a/src/managers/conda/condaPackageManager.ts b/src/managers/conda/condaPackageManager.ts index d4fb44be3..d395d0ce6 100644 --- a/src/managers/conda/condaPackageManager.ts +++ b/src/managers/conda/condaPackageManager.ts @@ -54,6 +54,10 @@ export class CondaPackageManager implements PackageManager, Disposable { let toUninstall: string[] = [...(options.uninstall ?? [])]; if (toInstall.length === 0 && toUninstall.length === 0) { + if (options.runHeadless) { + // Headless mode: skip the interactive package picker. + return; + } const result = await getCommonCondaPackagesToInstall(environment, options, this.api); if (result) { toInstall = result.install; @@ -91,9 +95,12 @@ export class CondaPackageManager implements PackageManager, Disposable { } this.log.error('Error installing packages', e); - setImmediate(async () => { - await showErrorMessageWithLogs(CondaStrings.condaInstallError, this.log); - }); + if (!manageOptions.runHeadless) { + setImmediate(async () => { + await showErrorMessageWithLogs(CondaStrings.condaInstallError, this.log); + }); + } + throw e; } }, ); diff --git a/src/managers/poetry/poetryPackageManager.ts b/src/managers/poetry/poetryPackageManager.ts index 9525254cb..e946f0452 100644 --- a/src/managers/poetry/poetryPackageManager.ts +++ b/src/managers/poetry/poetryPackageManager.ts @@ -59,6 +59,10 @@ export class PoetryPackageManager implements PackageManager, Disposable { let toUninstall: string[] = [...(options.uninstall ?? [])]; if (toInstall.length === 0 && toUninstall.length === 0) { + if (options.runHeadless) { + // Headless mode: skip the interactive package input prompt. + return; + } // Show package input UI if no packages are specified const installInput = await showInputBox({ prompt: 'Enter packages to install (comma separated)', @@ -99,12 +103,14 @@ export class PoetryPackageManager implements PackageManager, Disposable { throw e; } this.log.error('Error managing packages with Poetry', e); - setImmediate(async () => { - const result = await showErrorMessage('Error managing packages with Poetry', 'View Output'); - if (result === 'View Output') { - this.log.show(); - } - }); + if (!options.runHeadless) { + setImmediate(async () => { + const result = await showErrorMessage('Error managing packages with Poetry', 'View Output'); + if (result === 'View Output') { + this.log.show(); + } + }); + } throw e; } }, diff --git a/src/test/integration/packageManagement.integration.test.ts b/src/test/integration/packageManagement.integration.test.ts index 5998b6a17..7eb2a75c2 100644 --- a/src/test/integration/packageManagement.integration.test.ts +++ b/src/test/integration/packageManagement.integration.test.ts @@ -282,13 +282,13 @@ suite('Integration: Package Management', function () { try { if (wasInstalled) { // Uninstall first - await api.managePackages(targetEnv, { uninstall: [testPackage] }); + await api.managePackages(targetEnv, { uninstall: [testPackage], runHeadless: true }); packageInstalled = false; await sleep(2000); } // Install package - await api.managePackages(targetEnv, { install: [testPackage] }); + await api.managePackages(targetEnv, { install: [testPackage], runHeadless: true }); packageInstalled = true; // Refresh and verify @@ -299,7 +299,7 @@ suite('Integration: Package Management', function () { assert.ok(isNowInstalled, `${testPackage} should be installed after managePackages install`); // Uninstall - await api.managePackages(targetEnv, { uninstall: [testPackage] }); + await api.managePackages(targetEnv, { uninstall: [testPackage], runHeadless: true }); packageInstalled = false; // Refresh and verify @@ -312,7 +312,7 @@ suite('Integration: Package Management', function () { // Ensure cleanup even if assertions fail if (packageInstalled) { try { - await api.managePackages(targetEnv, { uninstall: [testPackage] }); + await api.managePackages(targetEnv, { uninstall: [testPackage], runHeadless: true }); } catch { console.log('Cleanup: failed to uninstall test package'); } diff --git a/src/test/integration/packageManager.integration.test.ts b/src/test/integration/packageManager.integration.test.ts new file mode 100644 index 000000000..f42df8539 --- /dev/null +++ b/src/test/integration/packageManager.integration.test.ts @@ -0,0 +1,271 @@ +import * as vscode from 'vscode'; + +import { compare } from '@renovatebot/pep440'; +import assert from 'assert'; +import * as path from 'path'; +import { Package, PythonEnvironment, PythonEnvironmentApi, PythonProject } from '../../api'; +import { CONDA_MANAGER_ID, DEFAULT_PACKAGE_MANAGER_ID, VENV_MANAGER_ID } from '../../common/constants'; +import { PythonProjectSettings } from '../../internal.api'; +import { getConda } from '../../managers/conda/condaUtils'; +import { ENVS_EXTENSION_ID } from '../constants'; +import { waitForCondition } from '../testUtils'; + +type PackageManagerId = `${string}:${string}`; + +interface PackageManagerProfile { + environmentManagerId: string; + name: string; + packageManagerId: PackageManagerId; + projectDirectory: string; + prerequisite(api: PythonEnvironmentApi): Promise; + supportsVersionLookup(packages: Package[]): boolean; +} + +const profiles: PackageManagerProfile[] = [ + { + environmentManagerId: VENV_MANAGER_ID, + name: 'Pip', + packageManagerId: DEFAULT_PACKAGE_MANAGER_ID, + projectDirectory: 'pip', + prerequisite: async (api) => + (await api.getEnvironments('global')).some((environment) => environment.version.startsWith('3.')), + supportsVersionLookup: (packages) => { + const pipVersion = packages.find((pkg) => pkg.name.toLowerCase() === 'pip')?.version; + return pipVersion !== undefined && compare(pipVersion, '21.2') >= 0; + }, + }, + { + environmentManagerId: CONDA_MANAGER_ID, + name: 'Conda', + packageManagerId: CONDA_MANAGER_ID, + projectDirectory: 'conda', + prerequisite: async () => { + try { + await getConda(); + return true; + } catch { + return false; + } + }, + supportsVersionLookup: () => true, + }, +]; + +const deferredPackageManagers: Readonly> = { + 'ms-python.python:poetry': 'Poetry lifecycle coverage requires a controlled Poetry installation.', +}; + +const deferredProfiles = { + pipWithUv: 'uv-backed Pip selection uses a machine-scoped setting and is unstable within one extension host.', +} as const; + +suite('Package Manager profile coverage', function () { + this.timeout(60_000); + + test('covers or explicitly defers every registered package manager', async () => { + const extension = vscode.extensions.getExtension(ENVS_EXTENSION_ID); + assert.ok(extension, 'Extension not found'); + const api: PythonEnvironmentApi = extension.isActive ? extension.exports : await extension.activate(); + await api.getEnvironments('global'); + + const registeredIds = await vscode.commands.executeCommand( + 'python-envs.test.getPackageManagerIds', + ); + assert.ok(registeredIds, 'Registered package-manager IDs are unavailable'); + + const coveredIds = new Set(profiles.map((profile) => profile.packageManagerId)); + const uncoveredIds = registeredIds.filter( + (managerId) => + !coveredIds.has(managerId as PackageManagerId) && + deferredPackageManagers[managerId as PackageManagerId] === undefined, + ); + assert.deepStrictEqual(uncoveredIds, [], `Package managers lack lifecycle coverage: ${uncoveredIds.join(', ')}`); + + for (const profile of profiles) { + assert.ok( + registeredIds.includes(profile.packageManagerId), + `Profile references an unregistered package manager: ${profile.packageManagerId}`, + ); + } + + for (const [profileName, reason] of Object.entries(deferredProfiles)) { + assert.ok(reason.length > 0, `Deferred profile lacks a reason: ${profileName}`); + } + }); +}); + +for (const profile of profiles) { + suite(`${profile.name} Package Manager`, function () { + this.timeout(300_000); + + let api: PythonEnvironmentApi; + let environment: PythonEnvironment | undefined; + let project: PythonProject | undefined; + let workspaceUri: vscode.Uri; + let previousPythonProjects: PythonProjectSettings[] | undefined; + let pythonProjectsUpdated = false; + suiteSetup(async function () { + if (process.env.VSC_PYTHON_PACKAGE_NETWORK_TEST !== '1') { + this.skip(); + return; + } + + const extension = vscode.extensions.getExtension(ENVS_EXTENSION_ID); + assert.ok(extension, 'Extension not found'); + if (!extension.isActive) { + await extension.activate(); + await waitForCondition(() => extension.isActive, 20_000, 'Extension did not activate in time'); + } + api = extension.exports; + assert.ok(api, 'API not available'); + + const workspaceFolder = vscode.workspace.workspaceFolders?.[0]; + assert.ok(workspaceFolder, 'Integration test workspace not found'); + workspaceUri = workspaceFolder.uri; + const config = vscode.workspace.getConfiguration('python-envs', workspaceUri); + + if (!(await profile.prerequisite(api))) { + this.skip(); + return; + } + + const projectUri = vscode.Uri.joinPath( + workspaceUri, + `.package-manager-test-${profile.projectDirectory}-${process.pid}`, + ); + await vscode.workspace.fs.createDirectory(projectUri); + project = { + name: `${profile.name} Package Manager Test`, + uri: projectUri, + }; + previousPythonProjects = config.inspect('pythonProjects')?.workspaceFolderValue; + const pythonProjects = config.get('pythonProjects', []); + const projectSetting: PythonProjectSettings = { + path: path.relative(workspaceUri.fsPath, projectUri.fsPath).replace(/\\/g, '/'), + envManager: profile.environmentManagerId, + packageManager: profile.packageManagerId, + workspace: workspaceFolder.name, + }; + await config.update( + 'pythonProjects', + [...pythonProjects, projectSetting], + vscode.ConfigurationTarget.WorkspaceFolder, + ); + pythonProjectsUpdated = true; + await waitForCondition( + () => + api + .getPythonProjects() + .some((registeredProject) => registeredProject.uri.toString() === projectUri.toString()), + 10_000, + `Python project was not registered: ${projectUri.fsPath}`, + ); + + await api.refreshEnvironments(projectUri); + + environment = await api.createEnvironment(projectUri, { quickCreate: true }); + assert.ok(environment, `${profile.name} failed to create an environment after prerequisites passed`); + assert.strictEqual( + environment.envId.managerId, + profile.environmentManagerId, + `Expected an environment created by ${profile.environmentManagerId}`, + ); + }); + + test(`${profile.name} Package Manager should install, list, and uninstall a package`, async () => { + const packageName = 'requests'; + const baseline = await api.getPackages(environment!, { skipCache: true }); + assert.ok(baseline, 'Unable to list packages before installation'); + const wasInstalled = baseline.some((pkg) => pkg.name.toLowerCase() === packageName); + + if (!wasInstalled) { + await api.managePackages(environment!, { install: [packageName], runHeadless: true }); + } + let packages = await api.getPackages(environment!, { skipCache: true }); + assert.ok(packages, 'Unable to list packages after installation'); + assert.ok( + packages.some((pkg) => pkg.name.toLowerCase() === packageName), + 'Package not installed', + ); + + const directPackageNames = await vscode.commands.executeCommand( + 'python-envs.test.getDirectPackageNames', + environment!, + ); + if (directPackageNames !== undefined) { + assert.ok(directPackageNames.includes(packageName), 'Installed package was not reported as direct'); + } + + if (!wasInstalled) { + await api.managePackages(environment!, { uninstall: [packageName], runHeadless: true }); + packages = await api.getPackages(environment!, { skipCache: true }); + assert.ok(packages, 'Unable to list packages after uninstallation'); + assert.ok( + !packages.some((pkg) => pkg.name.toLowerCase() === packageName), + 'Package not uninstalled', + ); + } + }); + + test(`${profile.name} Package Manager should list available package versions`, async function () { + const packages = await api.getPackages(environment!, { skipCache: true }); + assert.ok(packages, 'Unable to list packages before version lookup'); + if (!profile.supportsVersionLookup(packages)) { + this.skip(); + return; + } + + const versions = await api.getPackageAvailableVersions(environment!, 'requests'); + assert.ok(versions, `${profile.name} unexpectedly failed to retrieve package versions`); + assert.ok(versions.length > 0, 'No package versions available'); + }); + + suiteTeardown(async () => { + try { + if (environment) { + const environmentPath = environment.environmentPath; + await api.removeEnvironment(environment, { runHeadless: true }); + await assert.rejects( + async () => vscode.workspace.fs.stat(environmentPath), + (error: unknown) => + error instanceof vscode.FileSystemError && error.code === 'FileNotFound', + `Environment was not removed: ${environmentPath.fsPath}`, + ); + } + } finally { + const config = vscode.workspace.getConfiguration('python-envs', workspaceUri); + if (project) { + try { + await api.setEnvironment(project.uri, undefined); + } finally { + try { + if (pythonProjectsUpdated) { + await config.update( + 'pythonProjects', + previousPythonProjects, + vscode.ConfigurationTarget.WorkspaceFolder, + ); + await waitForCondition( + () => + !api + .getPythonProjects() + .some( + (registeredProject) => + registeredProject.uri.toString() === project!.uri.toString(), + ), + 10_000, + `Python project was not unregistered: ${project.uri.fsPath}`, + ); + } + } finally { + await vscode.workspace.fs.delete(project.uri, { + recursive: true, + useTrash: false, + }); + } + } + } + } + }); + }); +} diff --git a/src/test/managers/builtin/pipPackageManager.unit.test.ts b/src/test/managers/builtin/pipPackageManager.unit.test.ts index 8a64abf1b..549bdd2de 100644 --- a/src/test/managers/builtin/pipPackageManager.unit.test.ts +++ b/src/test/managers/builtin/pipPackageManager.unit.test.ts @@ -40,4 +40,24 @@ suite('PipPackageManager', () => { assert.deepStrictEqual(initial, [cachedPackage]); assert.deepStrictEqual(afterFailedRefresh, [cachedPackage]); }); + + test('preserves undefined when an uncached refresh fails', async () => { + const environment = { + envId: { id: 'test-environment', managerId: 'test-manager' }, + environmentPath: Uri.file('/path/to/environment'), + } as PythonEnvironment; + const manager = new PipPackageManager( + { createPackageItem: sinon.stub() } as unknown as PythonEnvironmentApi, + { error: sinon.stub(), info: sinon.stub() } as unknown as LogOutputChannel, + {} as VenvManager, + ); + const refreshPackages = sinon.stub(builtinUtils, 'refreshPipPackages').resolves(undefined); + + const firstResult = await manager.getPackages(environment); + const secondResult = await manager.getPackages(environment); + + assert.strictEqual(firstResult, undefined); + assert.strictEqual(secondResult, undefined); + assert.strictEqual(refreshPackages.callCount, 2, 'A failed refresh should not populate the package cache'); + }); }); diff --git a/src/test/managers/builtin/pipPackageRefresh.unit.test.ts b/src/test/managers/builtin/pipPackageRefresh.unit.test.ts new file mode 100644 index 000000000..dff10003c --- /dev/null +++ b/src/test/managers/builtin/pipPackageRefresh.unit.test.ts @@ -0,0 +1,53 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import { LogOutputChannel, Uri } from 'vscode'; +import { PythonEnvironment } from '../../../api'; +import * as errorUtils from '../../../common/errors/utils'; +import * as helpers from '../../../managers/builtin/helpers'; +import { refreshPipPackages } from '../../../managers/builtin/utils'; + +suite('Pip package refresh', () => { + let environment: PythonEnvironment; + let log: LogOutputChannel; + let showErrorMessageWithLogsStub: sinon.SinonStub; + + setup(() => { + environment = { + environmentPath: Uri.file('.'), + execInfo: { + run: { + executable: 'python', + }, + }, + } as PythonEnvironment; + log = { + error: sinon.stub(), + info: sinon.stub(), + } as unknown as LogOutputChannel; + + sinon.stub(helpers, 'shouldUseUv').resolves(false); + sinon.stub(helpers, 'runPython').rejects(new Error('pip list failed')); + showErrorMessageWithLogsStub = sinon.stub(errorUtils, 'showErrorMessageWithLogs').resolves(); + }); + + teardown(() => { + sinon.restore(); + }); + + test('shows an error when an interactive refresh fails', async () => { + const result = await refreshPipPackages(environment, log); + + assert.strictEqual(result, undefined); + assert.ok(showErrorMessageWithLogsStub.calledOnce); + }); + + test('does not show an error when a headless refresh fails', async () => { + const result = await refreshPipPackages(environment, log, { showErrors: false }); + + assert.strictEqual(result, undefined); + assert.ok(showErrorMessageWithLogsStub.notCalled); + }); +}); diff --git a/src/test/managers/builtin/pipVersions.unit.test.ts b/src/test/managers/builtin/pipVersions.unit.test.ts index 5c06c394b..b2bd15f6b 100644 --- a/src/test/managers/builtin/pipVersions.unit.test.ts +++ b/src/test/managers/builtin/pipVersions.unit.test.ts @@ -1,13 +1,16 @@ -import assert from 'assert'; import { explain } from '@renovatebot/pep440'; -import { parsePipIndexVersionsJson } from '../../../managers/builtin/pipPackageManager'; +import assert from 'assert'; +import { parsePipIndexVersionsJson, parsePipIndexVersionsText } from '../../../managers/builtin/pipPackageManager'; suite('Pip Version Parsing', () => { suite('parsePipIndexVersionsJson', () => { test('parses valid JSON with versions array', () => { const output = JSON.stringify({ name: 'requests', versions: ['2.31.0', '2.30.0', '2.29.0'] }); const versions = parsePipIndexVersionsJson(output); - assert.deepStrictEqual(versions, ['2.31.0', '2.30.0', '2.29.0'].map((v) => explain(v))); + assert.deepStrictEqual( + versions, + ['2.31.0', '2.30.0', '2.29.0'].map((v) => explain(v)), + ); }); test('parses output with a single version', () => { @@ -33,5 +36,29 @@ suite('Pip Version Parsing', () => { assert.strictEqual(versions, undefined); }); }); -}); + suite('parsePipIndexVersionsText', () => { + test('parses and sorts the available versions line', () => { + const output = [ + 'requests (2.32.5)', + 'Available versions: 2.31.0, 2.32.5, 2.30.0', + ' INSTALLED: 2.31.0', + ' LATEST: 2.32.5', + ].join('\n'); + const versions = parsePipIndexVersionsText(output); + assert.deepStrictEqual( + versions, + ['2.32.5', '2.31.0', '2.30.0'].map((version) => explain(version)), + ); + }); + + test('returns undefined when the available versions line is missing', () => { + assert.strictEqual(parsePipIndexVersionsText('ERROR: No matching distribution found'), undefined); + }); + + test('ignores invalid versions', () => { + const versions = parsePipIndexVersionsText('Available versions: invalid, 1.2.3'); + assert.deepStrictEqual(versions, [explain('1.2.3')]); + }); + }); +}); diff --git a/src/test/managers/builtin/venvManager.createRemove.unit.test.ts b/src/test/managers/builtin/venvManager.createRemove.unit.test.ts index 7e1e202be..12bf1c7b3 100644 --- a/src/test/managers/builtin/venvManager.createRemove.unit.test.ts +++ b/src/test/managers/builtin/venvManager.createRemove.unit.test.ts @@ -47,12 +47,11 @@ function createManager( const baseManager = { getEnvironments: sinon.stub().resolves(baseEnvironments), } as any as EnvironmentManager; - const manager = new VenvManager( - {} as NativePythonFinder, - api, - baseManager, - { info: sinon.stub(), error: sinon.stub(), warn: sinon.stub() } as any, - ); + const manager = new VenvManager({} as NativePythonFinder, api, baseManager, { + info: sinon.stub(), + error: sinon.stub(), + warn: sinon.stub(), + } as any); (manager as any)._initialized = { completed: true, promise: Promise.resolve() }; (manager as any).collection = []; return manager; @@ -221,6 +220,17 @@ suite('VenvManager.remove - orchestration', () => { assert.strictEqual(events[0][0].environment, env); }); + test('forwards headless removal options to the removal helper', async () => { + const manager = createManager(); + const env = environment(); + removeVenvStub.resolves(true); + + await manager.remove(env, { runHeadless: true }); + + assert.strictEqual(removeVenvStub.firstCall.args[0], env); + assert.deepStrictEqual(removeVenvStub.firstCall.args[2], { runHeadless: true }); + }); + test('does not mutate state when the removal helper returns false', async () => { const manager = createManager(); const env = environment(); diff --git a/src/test/managers/builtin/venvUtils.removeVenv.unit.test.ts b/src/test/managers/builtin/venvUtils.removeVenv.unit.test.ts index 068eb5dca..b1fae91bf 100644 --- a/src/test/managers/builtin/venvUtils.removeVenv.unit.test.ts +++ b/src/test/managers/builtin/venvUtils.removeVenv.unit.test.ts @@ -1,6 +1,13 @@ import * as assert from 'assert'; +import * as fs from 'fs-extra'; import * as os from 'os'; import * as path from 'path'; +import * as sinon from 'sinon'; +import * as windowApis from '../../../common/window.apis'; +import * as uvEnvironments from '../../../managers/builtin/uvEnvironments'; +import { removeVenv } from '../../../managers/builtin/venvUtils'; +import { createMockLogOutputChannel } from '../../mocks/helper'; +import { createMockPythonEnvironment } from '../../mocks/pythonEnvironment'; suite('venvUtils Path Validation', () => { suite('isDriveRoot behavior', () => { @@ -146,4 +153,28 @@ suite('venvUtils removeVenv validation integration', () => { 'Should check for pyvenv.cfg in the environment root', ); }); + + test('headless removal skips confirmation and removes the environment', async () => { + const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'remove-venv-')); + const envPath = path.join(tempRoot, '.venv'); + await fs.outputFile(path.join(envPath, 'pyvenv.cfg'), 'home = base'); + const showWarningMessageStub = sinon.stub(windowApis, 'showWarningMessage'); + sinon.stub(windowApis, 'withProgress').callsFake(async (_options, task) => task({} as never, {} as never)); + sinon.stub(uvEnvironments, 'removeUvEnvironment').resolves(); + + try { + const removed = await removeVenv( + createMockPythonEnvironment({ name: '.venv', envPath }), + createMockLogOutputChannel(), + { runHeadless: true }, + ); + + assert.strictEqual(removed, true); + assert.strictEqual(showWarningMessageStub.callCount, 0); + assert.strictEqual(await fs.pathExists(envPath), false); + } finally { + sinon.restore(); + await fs.remove(tempRoot); + } + }); }); diff --git a/src/test/managers/common/packageChanges.unit.test.ts b/src/test/managers/common/packageChanges.unit.test.ts index 1f65b3c75..8f6f77402 100644 --- a/src/test/managers/common/packageChanges.unit.test.ts +++ b/src/test/managers/common/packageChanges.unit.test.ts @@ -127,6 +127,35 @@ suite('packageChanges', () => { assert.strictEqual(changes[0].kind, PackageChangeKind.add); }); + test('uses an operation-specific package fetcher when provided', async () => { + const fetched = [{ name: 'requests', version: '2.31.0' } as Package]; + const fetchPackages = sinon.stub().resolves(fetched); + const onChanges = sinon.stub(); + + const result = await updatePackagesAndNotify( + packageManager, + environment, + undefined, + onChanges, + fetchPackages, + ); + + assert.deepStrictEqual(result, fetched); + assert.ok(fetchPackages.calledOnce); + assert.ok(getPackagesStub.notCalled); + }); + + test('preserves undefined and does not report removals when fetching fails', async () => { + const before = [{ name: 'requests', version: '2.31.0' } as Package]; + getPackagesStub.resolves(undefined); + const onChanges = sinon.stub(); + + const result = await updatePackagesAndNotify(packageManager, environment, before, onChanges); + + assert.strictEqual(result, undefined); + assert.ok(onChanges.notCalled); + }); + test('does not fire callback when nothing changed', async () => { const pkgs = [{ name: 'requests', version: '2.31.0' } as Package]; getPackagesStub.resolves(pkgs); diff --git a/src/test/managers/conda/condaPackageManager.unit.test.ts b/src/test/managers/conda/condaPackageManager.unit.test.ts new file mode 100644 index 000000000..ea6614daf --- /dev/null +++ b/src/test/managers/conda/condaPackageManager.unit.test.ts @@ -0,0 +1,41 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import { LogOutputChannel, Uri } from 'vscode'; +import { PythonEnvironment, PythonEnvironmentApi } from '../../../api'; +import * as errorUtils from '../../../common/errors/utils'; +import * as windowApis from '../../../common/window.apis'; +import { CondaPackageManager } from '../../../managers/conda/condaPackageManager'; +import * as condaUtils from '../../../managers/conda/condaUtils'; + +suite('CondaPackageManager', () => { + teardown(() => { + sinon.restore(); + }); + + test('headless package failures reject without showing error UI', async () => { + const environment = { + envId: { id: 'test-environment', managerId: 'test-manager' }, + environmentPath: Uri.file('/path/to/environment'), + } as PythonEnvironment; + const logError = sinon.stub(); + const log = { + error: logError, + } as unknown as LogOutputChannel; + const manager = new CondaPackageManager({} as PythonEnvironmentApi, log); + const operationError = new Error('conda install failed'); + sinon.stub(condaUtils, 'managePackages').rejects(operationError); + sinon.stub(windowApis, 'withProgress').callsFake(async (_options, task) => task({} as never, {} as never)); + const showErrorMessageWithLogs = sinon.stub(errorUtils, 'showErrorMessageWithLogs').resolves(); + + await assert.rejects( + manager.manage(environment, { install: ['requests'], runHeadless: true }), + (error: unknown) => error === operationError, + ); + + assert.ok(logError.calledOnce); + assert.ok(showErrorMessageWithLogs.notCalled); + }); +}); From 8666b65eec9366f9e1ca36994abad71d9ca8b8f3 Mon Sep 17 00:00:00 2001 From: Lee Murray Date: Tue, 18 Aug 2026 12:07:31 +0100 Subject: [PATCH 2/4] Update logo.svg with new design and optimized dimensions (#1719) Replace the existing activity bar icon with a new design more aligned with the wider codicon design language. ![image.png](https://github.com/user-attachments/assets/65b0c66b-0e1a-4610-89d7-94206fafa044) Co-authored-by: mrleemurray --- files/logo.svg | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/files/logo.svg b/files/logo.svg index a999dbeac..f849d01e2 100644 --- a/files/logo.svg +++ b/files/logo.svg @@ -1,14 +1,12 @@ - - - + + + + + + - - - - - - - - + + + From 59c1815c58fb27a5bf4b231ea08a4d17f31b5897 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Mon, 17 Aug 2026 12:59:13 -0700 Subject: [PATCH 3/4] Add inline script activation-time discovery Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3cb82ae9-7424-40a4-9156-8c54ac6e0895 --- src/common/inlineScript/cacheLayout.ts | 7 +- .../builtin/inlineScript/envManager.ts | 277 ++++++++++++++- src/managers/builtin/inlineScript/main.ts | 1 + .../inlineScript/cacheLayout.unit.test.ts | 47 +++ .../inlineScript/envManager.unit.test.ts | 318 +++++++++++++++++- .../builtin/inlineScript/main.unit.test.ts | 27 ++ 6 files changed, 673 insertions(+), 4 deletions(-) diff --git a/src/common/inlineScript/cacheLayout.ts b/src/common/inlineScript/cacheLayout.ts index 1371040b2..fdc31bf9f 100644 --- a/src/common/inlineScript/cacheLayout.ts +++ b/src/common/inlineScript/cacheLayout.ts @@ -204,7 +204,7 @@ export function selectStaleEntries(entries: ReadonlyArray, no } /** - * Verify that a cached env's base interpreter still exists on disk. + * Verify that a cached env's launcher and base interpreter still exist on disk. */ export async function verifyBaseInterpreterExists(envDir: Uri): Promise { return (await getBaseInterpreterStatus(envDir)) === 'available'; @@ -221,6 +221,11 @@ async function getPosixBaseInterpreterStatus(envDir: Uri): Promise { + const launcherStatus = await getRegularFileStatus(getVenvPythonPath(envDir.fsPath), 'cached interpreter launcher'); + if (launcherStatus !== 'available') { + return launcherStatus; + } + const pyvenvPath = Uri.joinPath(envDir, 'pyvenv.cfg').fsPath; let raw: string; try { diff --git a/src/managers/builtin/inlineScript/envManager.ts b/src/managers/builtin/inlineScript/envManager.ts index d68d9dda6..c7f6b6040 100644 --- a/src/managers/builtin/inlineScript/envManager.ts +++ b/src/managers/builtin/inlineScript/envManager.ts @@ -11,6 +11,7 @@ import { DidChangeEnvironmentEventArgs, DidChangeEnvironmentsEventArgs, EnvironmentManager, + EnvironmentChangeKind, GetEnvironmentScope, GetEnvironmentsScope, IconPath, @@ -49,6 +50,7 @@ import { normalizePath } from '../../../common/utils/pathUtils'; import { compareReleaseSegments, parseReleaseSegments } from '../../../common/utils/pep440Release'; import { getVenvPythonPath } from '../../../common/utils/virtualEnvironment'; import { NativePythonFinder } from '../../common/nativePythonFinder'; +import { sortEnvironments } from '../../common/utils'; import { resolveSystemPythonEnvironmentPath } from '../utils'; import * as uvPythonInstaller from '../uvPythonInstaller'; import { createWithProgress, resolveVenvPythonEnvironmentPath } from '../venvUtils'; @@ -62,6 +64,7 @@ const BASE_INTERPRETER_MANAGER_IDS = new Set([ const CACHE_LOCK_TIMEOUT_MS = 5 * 60 * 1000; const CACHE_LOCK_RETRY_MS = 500; const CACHED_ASSOCIATION_VALIDATION_INTERVAL_MS = 5_000; +const DISCOVERY_RETRY_DELAYS_MS = [1_000, 5_000] as const; /** Workspace-state key for PEP 723 script path to environment executable associations. */ export const INLINE_SCRIPT_ENVS_KEY = `${ENVS_EXTENSION_ID}:inline-script:SCRIPT_ENVIRONMENTS`; @@ -92,13 +95,19 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private readonly pendingCreations = new Map>(); private readonly directlyResolvedBaseInterpreters = new Map(); private baseInterpreterInstallationQueue: Promise = Promise.resolve(); + private collection: PythonEnvironment[] = []; private readonly pendingRehydrations = new Map>(); private readonly fsPathToEnv = new Map(); private readonly fsPathToPersistedEnvPath = new Map(); private readonly cachedAssociationValidatedAt = new Map(); private readonly associationRevisions = new Map(); + private pendingRefresh: Promise | undefined; + private activationDiscoveryActive = false; + private discoveryRetryAttempt = 0; + private discoveryRetryTimer: ReturnType | undefined; private persistenceQueue: Promise = Promise.resolve(); private selectionQueue: Promise = Promise.resolve(); + private disposed = false; private readonly _onDidChangeEnvironments = new EventEmitter(); public readonly onDidChangeEnvironments: Event = @@ -228,10 +237,17 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } async refresh(_scope: RefreshEnvironmentsScope): Promise { - return; + if (this.disposed) { + return; + } + this.stopActivationDiscovery(); + await this.getOrStartRefreshPass(); } - async getEnvironments(_scope: GetEnvironmentsScope): Promise { + async getEnvironments(scope: GetEnvironmentsScope): Promise { + if (scope === 'all') { + return Array.from(this.collection); + } return []; } @@ -247,6 +263,257 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return undefined; } + public startActivationDiscovery(): void { + if (this.disposed || this.activationDiscoveryActive) { + return; + } + this.activationDiscoveryActive = true; + this.discoveryRetryAttempt = 0; + this.runActivationDiscoveryPass(); + } + + private async getOrStartRefreshPass(): Promise { + const pending = this.pendingRefresh; + if (pending) { + return pending; + } + + const refresh = this.refreshDiscoveredEnvironments(); + this.pendingRefresh = refresh; + try { + return await refresh; + } finally { + if (this.pendingRefresh === refresh) { + this.pendingRefresh = undefined; + } + } + } + + private runActivationDiscoveryPass(): void { + if (this.disposed || !this.activationDiscoveryActive) { + return; + } + + void this.getOrStartRefreshPass() + .then((shouldRetry) => { + if (this.disposed || !this.activationDiscoveryActive) { + return; + } + if (!shouldRetry) { + this.stopActivationDiscovery(); + return; + } + this.scheduleActivationDiscoveryRetry(); + }) + .catch((error) => { + if (this.disposed || !this.activationDiscoveryActive) { + return; + } + this.log.warn(`Activation-time inline-script discovery failed: ${getErrorMessage(error)}`); + this.stopActivationDiscovery(); + }); + } + + private async refreshDiscoveredEnvironments(): Promise { + const cacheRoot = getScriptEnvCacheRoot(this.globalStorageUri); + const previousByKey = new Map( + this.collection.map((environment) => [this.getDiscoveredEnvironmentKey(environment), environment]), + ); + + let entryNames: string[]; + try { + entryNames = await fs.readdir(cacheRoot.fsPath); + } catch (error) { + if (this.isDefinitivelyStalePathError(error)) { + entryNames = []; + } else { + this.log.warn( + `Unable to inspect the inline-script cache root ${cacheRoot.fsPath}: ${getErrorMessage(error)}`, + ); + return true; + } + } + + const lockedKeys = new Set(); + const nextByKey = new Map(); + let shouldRetry = false; + for (const entryName of entryNames.sort()) { + if (entryName.endsWith('.lock')) { + lockedKeys.add(normalizePath(Uri.joinPath(cacheRoot, entryName.slice(0, -5)).fsPath)); + shouldRetry = true; + continue; + } + + if (this.disposed) { + return false; + } + + const envDir = Uri.joinPath(cacheRoot, entryName); + const key = normalizePath(envDir.fsPath); + const discovered = await this.inspectDiscoveredCacheEntry(cacheRoot, envDir); + if (discovered.kind === 'resolved') { + nextByKey.set(key, discovered.environment); + } else if (discovered.kind === 'preserve') { + shouldRetry = true; + const previous = previousByKey.get(key); + if (previous) { + nextByKey.set(key, previous); + } + } + } + for (const [key, previous] of previousByKey) { + if (!nextByKey.has(key) && lockedKeys.has(key)) { + nextByKey.set(key, previous); + } + } + + if (this.disposed) { + return false; + } + + // Preserve previously known entries when a refresh cannot safely classify + // them because a build is in progress or the filesystem is transiently unavailable. + this.replaceDiscoveredEnvironments(sortEnvironments(Array.from(nextByKey.values()))); + return shouldRetry; + } + + private async inspectDiscoveredCacheEntry( + cacheRoot: Uri, + envDir: Uri, + ): Promise { + try { + const stat = await fs.lstat(envDir.fsPath); + if (!stat.isDirectory() || stat.isSymbolicLink()) { + return { kind: 'skip' }; + } + } catch (error) { + return this.isDefinitivelyStalePathError(error) ? { kind: 'skip' } : { kind: 'preserve' }; + } + + if (await this.isCacheEntryBusy(envDir.fsPath)) { + return { kind: 'preserve' }; + } + + try { + if (!(await resolveCacheEntryPath(cacheRoot, envDir))) { + return { kind: 'skip' }; + } + } catch (error) { + return this.isDefinitivelyStalePathError(error) ? { kind: 'skip' } : { kind: 'preserve' }; + } + + const sidecarResult = await inspectMetaJson(envDir); + if (sidecarResult.kind !== 'valid') { + return { kind: sidecarResult.kind === 'unavailable' ? 'preserve' : 'skip' }; + } + + const baseInterpreterStatus = await getBaseInterpreterStatus(envDir); + if (baseInterpreterStatus !== 'available') { + return { kind: baseInterpreterStatus === 'unavailable' ? 'preserve' : 'skip' }; + } + + let environment: PythonEnvironment | undefined; + try { + environment = await resolveVenvPythonEnvironmentPath( + getVenvPythonPath(envDir.fsPath), + this.nativeFinder, + this.api, + this, + this.baseManager, + ); + } catch (error) { + this.log.warn( + `Unable to resolve inline-script cache entry ${envDir.fsPath}: ${getErrorMessage(error)}`, + ); + return { kind: 'preserve' }; + } + if (!environment) { + return { kind: 'preserve' }; + } + + const ownership = await inspectOwnedCacheEntry(environment, cacheRoot, envDir); + if (ownership !== 'expected') { + return { kind: ownership === 'uncertain' ? 'preserve' : 'skip' }; + } + if (!this.areEqualPythonReleases(environment.version, sidecarResult.metadata.baseInterpreterVersion)) { + return { kind: 'skip' }; + } + + return { kind: 'resolved', environment }; + } + + private replaceDiscoveredEnvironments(next: PythonEnvironment[]): void { + const previousByKey = new Map( + this.collection.map((environment) => [this.getDiscoveredEnvironmentKey(environment), environment]), + ); + const nextByKey = new Map(next.map((environment) => [this.getDiscoveredEnvironmentKey(environment), environment])); + const changes: DidChangeEnvironmentsEventArgs = []; + + for (const [key, previous] of previousByKey) { + const current = nextByKey.get(key); + if (!current || !this.isSameDiscoveredEnvironment(previous, current)) { + changes.push({ kind: EnvironmentChangeKind.remove, environment: previous }); + } + } + for (const [key, current] of nextByKey) { + const previous = previousByKey.get(key); + if (!previous || !this.isSameDiscoveredEnvironment(previous, current)) { + changes.push({ kind: EnvironmentChangeKind.add, environment: current }); + } + } + + this.collection = next; + if (changes.length > 0) { + this._onDidChangeEnvironments.fire(changes); + } + } + + private getDiscoveredEnvironmentKey(environment: PythonEnvironment): string { + return normalizePath(environment.sysPrefix); + } + + private isSameDiscoveredEnvironment(first: PythonEnvironment, second: PythonEnvironment): boolean { + return ( + first.envId.managerId === second.envId.managerId && + normalizePath(first.environmentPath.fsPath) === normalizePath(second.environmentPath.fsPath) && + first.version === second.version + ); + } + + private scheduleActivationDiscoveryRetry(): void { + if (this.discoveryRetryTimer) { + return; + } + + const delayMs = this.getDiscoveryRetryDelayMs(this.discoveryRetryAttempt); + if (delayMs === undefined) { + this.stopActivationDiscovery(); + return; + } + + this.discoveryRetryAttempt += 1; + this.discoveryRetryTimer = setTimeout(() => { + this.discoveryRetryTimer = undefined; + if (this.disposed || !this.activationDiscoveryActive) { + return; + } + this.runActivationDiscoveryPass(); + }, delayMs); + } + + private getDiscoveryRetryDelayMs(attempt: number): number | undefined { + return DISCOVERY_RETRY_DELAYS_MS[attempt]; + } + + private stopActivationDiscovery(): void { + if (this.discoveryRetryTimer) { + clearTimeout(this.discoveryRetryTimer); + this.discoveryRetryTimer = undefined; + } + this.activationDiscoveryActive = false; + this.discoveryRetryAttempt = 0; + } + private getScriptUri(scope: CreateEnvironmentScope): Uri | undefined { const uri = scope instanceof Uri ? scope : Array.isArray(scope) && scope.length === 1 ? scope[0] : undefined; return uri?.scheme === 'file' ? uri : undefined; @@ -1283,6 +1550,8 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } dispose(): void { + this.disposed = true; + this.stopActivationDiscovery(); this._onDidChangeEnvironments.dispose(); this._onDidChangeEnvironment.dispose(); } @@ -1306,3 +1575,7 @@ interface PendingScriptUpdate extends ScriptReference { readonly needsPersistence: boolean; readonly shouldNotify: boolean; } + +type DiscoveredCacheEntryResult = + | { readonly kind: 'preserve' | 'skip' } + | { readonly kind: 'resolved'; readonly environment: PythonEnvironment }; diff --git a/src/managers/builtin/inlineScript/main.ts b/src/managers/builtin/inlineScript/main.ts index 8c35fc6ed..46760ff72 100644 --- a/src/managers/builtin/inlineScript/main.ts +++ b/src/managers/builtin/inlineScript/main.ts @@ -29,5 +29,6 @@ export async function registerInlineScriptFeatures( const api: PythonEnvironmentApi = await getPythonApi(); const mgr = new InlineScriptEnvManager(nativeFinder, api, baseManager, globalStorageUri, log); disposables.push(mgr, api.registerEnvironmentManager(mgr)); + setImmediate(() => mgr.startActivationDiscovery()); traceInfo('Inline-script env manager: registered (internal flag is on)'); } diff --git a/src/test/common/inlineScript/cacheLayout.unit.test.ts b/src/test/common/inlineScript/cacheLayout.unit.test.ts index d57be848b..ce63d1c49 100644 --- a/src/test/common/inlineScript/cacheLayout.unit.test.ts +++ b/src/test/common/inlineScript/cacheLayout.unit.test.ts @@ -640,18 +640,60 @@ suite('inlineScriptCacheLayout', () => { await fs.writeFile(path.join(envDir.fsPath, 'pyvenv.cfg'), content, 'utf8'); } + async function writeLauncher(): Promise { + const launcherPath = getVenvPythonPath(envDir.fsPath); + await fs.ensureDir(path.dirname(launcherPath)); + await fs.writeFile(launcherPath, ''); + return launcherPath; + } + test('returns true when pyvenv.cfg.home points to an existing python.exe', async () => { const homeDir = path.join(tmpDir, 'Python313'); await fs.ensureDir(homeDir); await fs.writeFile(path.join(homeDir, 'python.exe'), ''); + await writeLauncher(); await writePyvenvCfg(`home = ${homeDir}\ninclude-system-site-packages = false\nversion = 3.13.0\n`); assert.strictEqual(await verifyBaseInterpreterExists(envDir), true); assert.strictEqual(traceWarnStub.called, false, 'no warn on success'); }); + test('returns false when the cached launcher is missing even if the base python still exists', async () => { + const homeDir = path.join(tmpDir, 'Python313'); + await fs.ensureDir(homeDir); + await fs.writeFile(path.join(homeDir, 'python.exe'), ''); + await writePyvenvCfg(`home = ${homeDir}\n`); + assert.strictEqual(await getBaseInterpreterStatus(envDir), 'missing'); + assert.strictEqual(await verifyBaseInterpreterExists(envDir), false); + assert.ok( + traceWarnStub.getCalls().some((c) => String(c.args[0]).includes('cached interpreter launcher')), + 'expected a missing-launcher warn', + ); + }); + + test('returns false when the cached launcher path is not a regular file', async () => { + const homeDir = path.join(tmpDir, 'Python313'); + await fs.ensureDir(homeDir); + await fs.writeFile(path.join(homeDir, 'python.exe'), ''); + const launcherPath = getVenvPythonPath(envDir.fsPath); + await fs.ensureDir(launcherPath); + await writePyvenvCfg(`home = ${homeDir}\n`); + assert.strictEqual(await getBaseInterpreterStatus(envDir), 'missing'); + assert.strictEqual(await verifyBaseInterpreterExists(envDir), false); + assert.ok( + traceWarnStub.getCalls().some((c) => String(c.args[0]).includes('not a regular file')), + 'expected a non-file launcher warn', + ); + }); + + test('classifies a transient cached-launcher stat failure as unavailable', async () => { + sinon.stub(fsExtra, 'stat').rejects(Object.assign(new Error('I/O error'), { code: 'EIO' })); + assert.strictEqual(await getBaseInterpreterStatus(envDir), 'unavailable'); + }); + test('returns false when pyvenv.cfg.home points to a removed python.exe', async () => { const homeDir = path.join(tmpDir, 'Python313'); await fs.ensureDir(homeDir); + await writeLauncher(); await writePyvenvCfg(`home = ${homeDir}\n`); assert.strictEqual(await verifyBaseInterpreterExists(envDir), false); assert.ok( @@ -661,6 +703,7 @@ suite('inlineScriptCacheLayout', () => { }); test('returns false when pyvenv.cfg is missing entirely', async () => { + await writeLauncher(); assert.strictEqual(await verifyBaseInterpreterExists(envDir), false); assert.ok( traceWarnStub @@ -671,6 +714,7 @@ suite('inlineScriptCacheLayout', () => { }); test('returns false when pyvenv.cfg has no `home =` line', async () => { + await writeLauncher(); await writePyvenvCfg('include-system-site-packages = false\nversion = 3.13.0\n'); assert.strictEqual(await verifyBaseInterpreterExists(envDir), false); assert.ok( @@ -679,6 +723,7 @@ suite('inlineScriptCacheLayout', () => { }); test('returns false when pyvenv.cfg has an empty home value', async () => { + await writeLauncher(); await writePyvenvCfg('home =\n'); assert.strictEqual(await verifyBaseInterpreterExists(envDir), false); assert.ok( @@ -690,6 +735,7 @@ suite('inlineScriptCacheLayout', () => { const homeDir = path.join(tmpDir, 'Python313'); await fs.ensureDir(homeDir); await fs.writeFile(path.join(homeDir, 'python.exe'), ''); + await writeLauncher(); await writePyvenvCfg(` home = ${homeDir} \n`); assert.strictEqual(await verifyBaseInterpreterExists(envDir), true); }); @@ -698,6 +744,7 @@ suite('inlineScriptCacheLayout', () => { const homeDir = path.join(tmpDir, 'Python313'); await fs.ensureDir(homeDir); await fs.writeFile(path.join(homeDir, 'python.exe'), ''); + await writeLauncher(); await writePyvenvCfg(`home = ${homeDir}\r\nversion = 3.13.0\r\n`); assert.strictEqual(await verifyBaseInterpreterExists(envDir), true); }); diff --git a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts index 3d0488cae..4daf64d6d 100644 --- a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts @@ -7,7 +7,7 @@ import * as os from 'os'; import * as path from 'path'; import * as sinon from 'sinon'; import { LogOutputChannel, Uri } from 'vscode'; -import { EnvironmentManager, PythonEnvironment, PythonEnvironmentApi } from '../../../../api'; +import { EnvironmentChangeKind, EnvironmentManager, PythonEnvironment, PythonEnvironmentApi } from '../../../../api'; import * as cacheKey from '../../../../common/inlineScript/cacheKey'; import * as cacheLayout from '../../../../common/inlineScript/cacheLayout'; import * as metadataReader from '../../../../common/inlineScript/metadata'; @@ -201,6 +201,27 @@ suite('InlineScriptEnvManager', () => { inspectMetaStub.resolves({ kind: 'valid', metadata }); } + async function makeSidecar( + overrides: Partial = {}, + ): Promise { + return { + schemaVersion: cacheLayout.META_SCHEMA_VERSION, + baseInterpreterPath: await fs.realpath(baseExecutable), + baseInterpreterVersion: baseEnvironment.version, + lastUsedAt: NOW.toISOString(), + ...overrides, + }; + } + + function setSidecarResults(results: Record): void { + inspectMetaStub.callsFake(async (candidate: Uri) => results[path.basename(candidate.fsPath)] ?? { kind: 'missing' }); + } + + function setResolvedVenvs(environments: readonly PythonEnvironment[]): void { + const byPath = new Map(environments.map((environment) => [normalizePath(environment.environmentPath.fsPath), environment])); + resolveVenvStub.callsFake(async (candidatePath: string) => byPath.get(normalizePath(candidatePath))); + } + async function createOwnedEnvironment( cacheKey: string = CACHE_KEY, envId: string = `inline-${cacheKey}`, @@ -224,6 +245,29 @@ suite('InlineScriptEnvManager', () => { assert.fail('Expected the stub to be called'); } + async function waitForStubCallCount(stub: sinon.SinonStub, expectedCallCount: number): Promise { + for (let attempt = 0; attempt < 20; attempt += 1) { + if (stub.callCount >= expectedCallCount) { + return; + } + await new Promise((resolve) => setTimeout(resolve, 5)); + } + assert.fail(`Expected the stub to be called ${expectedCallCount} times`); + } + + async function waitForCondition( + predicate: () => boolean | Promise, + message: string, + ): Promise { + for (let attempt = 0; attempt < 20; attempt += 1) { + if (await predicate()) { + return; + } + await new Promise((resolve) => setTimeout(resolve, 5)); + } + assert.fail(message); + } + function nextTurn(): Promise { return new Promise((resolve) => setImmediate(resolve)); } @@ -1465,6 +1509,278 @@ suite('InlineScriptEnvManager', () => { }); }); + suite('activation-time discovery', () => { + test('cold-start transient resolution retries and later discovers the environment', async () => { + const environment = await createOwnedEnvironment(); + const sidecar = await makeSidecar(); + setSidecarResults({ [CACHE_KEY]: { kind: 'valid', metadata: sidecar } }); + resolveVenvStub.onFirstCall().resolves(undefined); + resolveVenvStub.onSecondCall().resolves(environment); + const retryManager = manager as unknown as { + getDiscoveryRetryDelayMs(attempt: number): number | undefined; + }; + sinon.stub(retryManager, 'getDiscoveryRetryDelayMs').callsFake((attempt) => (attempt === 0 ? 0 : undefined)); + const listener = sinon.spy(); + manager.onDidChangeEnvironments(listener); + + manager.startActivationDiscovery(); + + await waitForStubCallCount(resolveVenvStub, 2); + await waitForCondition( + async () => (await manager.getEnvironments('all')).length === 1, + 'Expected the follow-up discovery retry to publish the environment', + ); + assert.deepStrictEqual(await manager.getEnvironments('all'), [environment]); + assert.deepStrictEqual(listener.firstCall.args[0], [ + { kind: EnvironmentChangeKind.add, environment }, + ]); + }); + + test('refresh discovers valid cached environments and exposes them only through all-scope', async () => { + const first = await createOwnedEnvironment(); + const secondKey = 'fedcba9876543210'; + const second = await createOwnedEnvironment(secondKey); + const sidecar = await makeSidecar(); + setSidecarResults({ + [CACHE_KEY]: { kind: 'valid', metadata: sidecar }, + [secondKey]: { kind: 'valid', metadata: sidecar }, + }); + setResolvedVenvs([first, second]); + const listener = sinon.spy(); + manager.onDidChangeEnvironments(listener); + + await manager.refresh(undefined); + + const discovered = await manager.getEnvironments('all'); + assert.deepStrictEqual( + discovered.map((environment) => normalizePath(environment.sysPrefix)).sort(), + [first, second].map((environment) => normalizePath(environment.sysPrefix)).sort(), + ); + assert.deepStrictEqual(await manager.getEnvironments('global'), []); + assert.strictEqual(listener.callCount, 1); + assert.deepStrictEqual( + listener.firstCall.args[0].map((change: { kind: EnvironmentChangeKind }) => change.kind), + [EnvironmentChangeKind.add, EnvironmentChangeKind.add], + ); + }); + + test('refresh skips missing, invalid, unavailable, and non-directory cache entries', async () => { + const valid = await createOwnedEnvironment(); + const cacheRoot = cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath; + const invalidKey = 'invalid-sidecar'; + const unavailableKey = 'unavailable-sidecar'; + await fs.ensureDir(path.join(cacheRoot, 'missing-sidecar')); + await fs.ensureDir(path.join(cacheRoot, invalidKey)); + await fs.ensureDir(path.join(cacheRoot, unavailableKey)); + await fs.outputFile(path.join(cacheRoot, 'not-a-directory'), ''); + const sidecar = await makeSidecar(); + setSidecarResults({ + [CACHE_KEY]: { kind: 'valid', metadata: sidecar }, + [invalidKey]: { kind: 'invalid' }, + [unavailableKey]: { kind: 'unavailable' }, + }); + setResolvedVenvs([valid]); + + await manager.refresh(undefined); + + const discovered = await manager.getEnvironments('all'); + assert.deepStrictEqual(discovered, [valid]); + assert.strictEqual(resolveVenvStub.callCount, 1); + }); + + test('refresh preserves a previously discovered environment while its cache entry is locked', async () => { + const environment = await createOwnedEnvironment(); + const sidecar = await makeSidecar(); + setSidecarResults({ [CACHE_KEY]: { kind: 'valid', metadata: sidecar } }); + setResolvedVenvs([environment]); + await manager.refresh(undefined); + resolveVenvStub.resetHistory(); + const listener = sinon.spy(); + manager.onDidChangeEnvironments(listener); + await fs.remove(environment.sysPrefix); + await fs.ensureDir(`${path.resolve(environment.sysPrefix)}.lock`); + + await manager.refresh(undefined); + + assert.deepStrictEqual(await manager.getEnvironments('all'), [environment]); + assert.strictEqual(resolveVenvStub.callCount, 0); + assert.strictEqual(listener.callCount, 0); + }); + + test('refresh removes a previously discovered environment when launcher inspection marks it missing', async () => { + const environment = await createOwnedEnvironment(); + const sidecar = await makeSidecar(); + setSidecarResults({ [CACHE_KEY]: { kind: 'valid', metadata: sidecar } }); + setResolvedVenvs([environment]); + await manager.refresh(undefined); + resolveVenvStub.resetHistory(); + baseInterpreterStatusStub.resolves('missing'); + const listener = sinon.spy(); + manager.onDidChangeEnvironments(listener); + + await manager.refresh(undefined); + + assert.deepStrictEqual(await manager.getEnvironments('all'), []); + assert.strictEqual(resolveVenvStub.callCount, 0); + assert.deepStrictEqual(listener.firstCall.args[0], [ + { kind: EnvironmentChangeKind.remove, environment }, + ]); + }); + + test('coalesces concurrent refresh requests for the same scan', async () => { + const environment = await createOwnedEnvironment(); + const sidecar = await makeSidecar(); + setSidecarResults({ [CACHE_KEY]: { kind: 'valid', metadata: sidecar } }); + let resolveDiscovery: ((value: PythonEnvironment | undefined) => void) | undefined; + resolveVenvStub.callsFake( + () => + new Promise((resolve) => { + resolveDiscovery = resolve; + }), + ); + const listener = sinon.spy(); + manager.onDidChangeEnvironments(listener); + + const firstRefresh = manager.refresh(undefined); + const secondRefresh = manager.refresh(undefined); + await waitForStubCall(resolveVenvStub); + assert.strictEqual(resolveVenvStub.callCount, 1); + resolveDiscovery!(environment); + await Promise.all([firstRefresh, secondRefresh]); + + assert.deepStrictEqual(await manager.getEnvironments('all'), [environment]); + assert.strictEqual(listener.callCount, 1); + }); + + test('explicit refresh does not schedule a delayed follow-up after an uncertain pass', async () => { + const sidecar = await makeSidecar(); + await createOwnedEnvironment(); + setSidecarResults({ [CACHE_KEY]: { kind: 'valid', metadata: sidecar } }); + resolveVenvStub.resolves(undefined); + const retryManager = manager as unknown as { + getDiscoveryRetryDelayMs(attempt: number): number | undefined; + }; + sinon.stub(retryManager, 'getDiscoveryRetryDelayMs').returns(0); + + await manager.refresh(undefined); + await new Promise((resolve) => setTimeout(resolve, 25)); + + assert.strictEqual(resolveVenvStub.callCount, 1); + assert.deepStrictEqual(await manager.getEnvironments('all'), []); + }); + + test('explicit refresh overlapping bootstrap cancels the later activation retry', async () => { + const sidecar = await makeSidecar(); + await createOwnedEnvironment(); + setSidecarResults({ [CACHE_KEY]: { kind: 'valid', metadata: sidecar } }); + let resolveDiscovery: ((value: PythonEnvironment | undefined) => void) | undefined; + resolveVenvStub.onFirstCall().callsFake( + () => + new Promise((resolve) => { + resolveDiscovery = resolve; + }), + ); + resolveVenvStub.onSecondCall().resolves(undefined); + const retryManager = manager as unknown as { + getDiscoveryRetryDelayMs(attempt: number): number | undefined; + }; + sinon.stub(retryManager, 'getDiscoveryRetryDelayMs').returns(0); + + manager.startActivationDiscovery(); + await waitForStubCall(resolveVenvStub); + + const refresh = manager.refresh(undefined); + resolveDiscovery!(undefined); + await refresh; + await new Promise((resolve) => setTimeout(resolve, 25)); + + assert.strictEqual(resolveVenvStub.callCount, 1); + assert.deepStrictEqual(await manager.getEnvironments('all'), []); + }); + + test('stops retrying after the bounded follow-up attempts', async () => { + const sidecar = await makeSidecar(); + await createOwnedEnvironment(); + setSidecarResults({ [CACHE_KEY]: { kind: 'valid', metadata: sidecar } }); + resolveVenvStub.resolves(undefined); + const retryManager = manager as unknown as { + getDiscoveryRetryDelayMs(attempt: number): number | undefined; + }; + sinon.stub(retryManager, 'getDiscoveryRetryDelayMs').callsFake((attempt) => (attempt < 2 ? 0 : undefined)); + + manager.startActivationDiscovery(); + await waitForStubCallCount(resolveVenvStub, 3); + await new Promise((resolve) => setTimeout(resolve, 25)); + + assert.strictEqual(resolveVenvStub.callCount, 3); + assert.deepStrictEqual(await manager.getEnvironments('all'), []); + }); + + test('refresh fires remove events when a discovered cache entry becomes invalid', async () => { + const environment = await createOwnedEnvironment(); + let sidecarResult: cacheLayout.InlineScriptMetaReadResult = { + kind: 'valid', + metadata: await makeSidecar(), + }; + inspectMetaStub.callsFake(async () => sidecarResult); + setResolvedVenvs([environment]); + await manager.refresh(undefined); + const listener = sinon.spy(); + manager.onDidChangeEnvironments(listener); + sidecarResult = { kind: 'invalid' }; + + await manager.refresh(undefined); + + assert.deepStrictEqual(await manager.getEnvironments('all'), []); + assert.strictEqual(listener.callCount, 1); + assert.deepStrictEqual(listener.firstCall.args[0], [ + { kind: EnvironmentChangeKind.remove, environment }, + ]); + }); + + test('does not publish discovery results after disposal while refresh is in flight', async () => { + const environment = await createOwnedEnvironment(); + const sidecar = await makeSidecar(); + setSidecarResults({ [CACHE_KEY]: { kind: 'valid', metadata: sidecar } }); + let resolveDiscovery: ((value: PythonEnvironment | undefined) => void) | undefined; + resolveVenvStub.callsFake( + () => + new Promise((resolve) => { + resolveDiscovery = resolve; + }), + ); + const listener = sinon.spy(); + manager.onDidChangeEnvironments(listener); + + const refresh = manager.refresh(undefined); + await waitForStubCall(resolveVenvStub); + manager.dispose(); + resolveDiscovery!(environment); + await refresh; + + assert.deepStrictEqual(await manager.getEnvironments('all'), []); + assert.strictEqual(listener.callCount, 0); + }); + + test('dispose cancels a pending discovery retry', async () => { + const sidecar = await makeSidecar(); + await createOwnedEnvironment(); + setSidecarResults({ [CACHE_KEY]: { kind: 'valid', metadata: sidecar } }); + resolveVenvStub.resolves(undefined); + const retryManager = manager as unknown as { + getDiscoveryRetryDelayMs(attempt: number): number | undefined; + }; + sinon.stub(retryManager, 'getDiscoveryRetryDelayMs').returns(25); + + manager.startActivationDiscovery(); + await waitForStubCall(resolveVenvStub); + manager.dispose(); + await new Promise((resolve) => setTimeout(resolve, 40)); + + assert.strictEqual(resolveVenvStub.callCount, 1); + }); + }); + suite('events and disposal', () => { test('create does not establish an association or fire later-phase events', async () => { const environmentsListener = sinon.spy(); diff --git a/src/test/managers/builtin/inlineScript/main.unit.test.ts b/src/test/managers/builtin/inlineScript/main.unit.test.ts index d109e318d..3531930fd 100644 --- a/src/test/managers/builtin/inlineScript/main.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/main.unit.test.ts @@ -7,6 +7,7 @@ import { Disposable, LogOutputChannel, Uri } from 'vscode'; import { EnvironmentManager, PythonEnvironmentApi } from '../../../../api'; import * as pythonApi from '../../../../features/pythonApi'; import * as helpers from '../../../../helpers'; +import { InlineScriptEnvManager } from '../../../../managers/builtin/inlineScript/envManager'; import { registerInlineScriptFeatures } from '../../../../managers/builtin/inlineScript/main'; import { NativePythonFinder } from '../../../../managers/common/nativePythonFinder'; @@ -27,10 +28,15 @@ function makeFakeLog(): LogOutputChannel { } as unknown as LogOutputChannel; } +function nextTurn(): Promise { + return new Promise((resolve) => setImmediate(resolve)); +} + suite('registerInlineScriptFeatures (feature-flag gate)', () => { let isEnabledStub: sinon.SinonStub; let getPythonApiStub: sinon.SinonStub; let registerEnvironmentManagerStub: sinon.SinonStub; + let startActivationDiscoveryStub: sinon.SinonStub; const nativeFinder = {} as NativePythonFinder; const baseManager = {} as EnvironmentManager; const globalStorageUri = Uri.file('inline-script-global-storage'); @@ -38,6 +44,7 @@ suite('registerInlineScriptFeatures (feature-flag gate)', () => { setup(() => { isEnabledStub = sinon.stub(helpers, 'isInlineScriptsFeatureEnabled'); registerEnvironmentManagerStub = sinon.stub<[unknown], Disposable>().returns({ dispose: () => undefined }); + startActivationDiscoveryStub = sinon.stub(InlineScriptEnvManager.prototype, 'startActivationDiscovery'); getPythonApiStub = sinon.stub(pythonApi, 'getPythonApi').resolves({ registerEnvironmentManager: registerEnvironmentManagerStub, } as unknown as PythonEnvironmentApi); @@ -74,5 +81,25 @@ suite('registerInlineScriptFeatures (feature-flag gate)', () => { 'registration disposable should be disposed', ); assert.strictEqual(typeof manager.create, 'function'); + await nextTurn(); + disposables.forEach((disposable) => disposable.dispose()); + }); + + test('when the feature flag is TRUE: defers activation-time discovery to the next turn', async () => { + isEnabledStub.returns(true); + const disposables: Disposable[] = []; + + await registerInlineScriptFeatures(nativeFinder, disposables, makeFakeLog(), baseManager, globalStorageUri); + + assert.strictEqual( + startActivationDiscoveryStub.callCount, + 0, + 'activation should not synchronously start bootstrap discovery', + ); + + await nextTurn(); + + sinon.assert.calledOnceWithExactly(startActivationDiscoveryStub); + disposables.forEach((disposable) => disposable.dispose()); }); }); From faa729f56df46092ff3558aa413d1a5692cfe911 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Mon, 17 Aug 2026 17:22:21 -0700 Subject: [PATCH 4/4] Harden inline script activation discovery Use stable cache identities, fail closed on lock probes, and retry snapshot changes safely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6b12d843-8011-4bfc-9ba9-f75761eadee2 --- .../builtin/inlineScript/envManager.ts | 129 ++++++++-- .../inlineScript/envManager.unit.test.ts | 224 ++++++++++++++++++ 2 files changed, 332 insertions(+), 21 deletions(-) diff --git a/src/managers/builtin/inlineScript/envManager.ts b/src/managers/builtin/inlineScript/envManager.ts index c7f6b6040..eedc5b41b 100644 --- a/src/managers/builtin/inlineScript/envManager.ts +++ b/src/managers/builtin/inlineScript/envManager.ts @@ -64,7 +64,7 @@ const BASE_INTERPRETER_MANAGER_IDS = new Set([ const CACHE_LOCK_TIMEOUT_MS = 5 * 60 * 1000; const CACHE_LOCK_RETRY_MS = 500; const CACHED_ASSOCIATION_VALIDATION_INTERVAL_MS = 5_000; -const DISCOVERY_RETRY_DELAYS_MS = [1_000, 5_000] as const; +const DISCOVERY_RETRY_DELAYS_MS = [1_000, 5_000, 30_000] as const; /** Workspace-state key for PEP 723 script path to environment executable associations. */ export const INLINE_SCRIPT_ENVS_KEY = `${ENVS_EXTENSION_ID}:inline-script:SCRIPT_ENVIRONMENTS`; @@ -85,6 +85,11 @@ interface BuildCacheEntryResult { readonly retainLock?: boolean; } +interface DiscoveryRefreshPass { + readonly promise: Promise; + readonly checksForSnapshotChanges: boolean; +} + type CacheEntryInspection = | { readonly kind: 'absent' | 'stale' | 'uncertain' } | { readonly kind: 'reusable'; readonly environment: PythonEnvironment }; @@ -101,7 +106,8 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { private readonly fsPathToPersistedEnvPath = new Map(); private readonly cachedAssociationValidatedAt = new Map(); private readonly associationRevisions = new Map(); - private pendingRefresh: Promise | undefined; + private pendingRefresh: DiscoveryRefreshPass | undefined; + private pendingSnapshotRefresh: Promise | undefined; private activationDiscoveryActive = false; private discoveryRetryAttempt = 0; private discoveryRetryTimer: ReturnType | undefined; @@ -241,7 +247,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return; } this.stopActivationDiscovery(); - await this.getOrStartRefreshPass(); + await this.getOrStartRefreshPass(false); } async getEnvironments(scope: GetEnvironmentsScope): Promise { @@ -272,21 +278,74 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { this.runActivationDiscoveryPass(); } - private async getOrStartRefreshPass(): Promise { + private getOrStartRefreshPass(checkForSnapshotChanges: boolean): Promise { const pending = this.pendingRefresh; + if (pending) { + return checkForSnapshotChanges && !pending.checksForSnapshotChanges + ? this.getOrScheduleSnapshotRefresh(pending) + : pending.promise; + } + + return this.startRefreshPass(checkForSnapshotChanges); + } + + private startRefreshPass(checkForSnapshotChanges: boolean): Promise { + const pass: DiscoveryRefreshPass = { + promise: this.refreshDiscoveredEnvironments(checkForSnapshotChanges), + checksForSnapshotChanges: checkForSnapshotChanges, + }; + this.pendingRefresh = pass; + void pass.promise.then( + () => { + if (this.pendingRefresh === pass) { + this.pendingRefresh = undefined; + } + }, + () => { + if (this.pendingRefresh === pass) { + this.pendingRefresh = undefined; + } + }, + ); + return pass.promise; + } + + private getOrScheduleSnapshotRefresh(sharedPass: DiscoveryRefreshPass): Promise { + const pending = this.pendingSnapshotRefresh; if (pending) { return pending; } - const refresh = this.refreshDiscoveredEnvironments(); - this.pendingRefresh = refresh; - try { - return await refresh; - } finally { - if (this.pendingRefresh === refresh) { - this.pendingRefresh = undefined; + const followUp = this.startSnapshotRefreshAfter(sharedPass); + this.pendingSnapshotRefresh = followUp; + void followUp.then( + () => { + if (this.pendingSnapshotRefresh === followUp) { + this.pendingSnapshotRefresh = undefined; + } + }, + () => { + if (this.pendingSnapshotRefresh === followUp) { + this.pendingSnapshotRefresh = undefined; + } + }, + ); + return followUp; + } + + private startSnapshotRefreshAfter(sharedPass: DiscoveryRefreshPass): Promise { + return sharedPass.promise.then(() => { + if (this.disposed || !this.activationDiscoveryActive) { + return false; } - } + const pending = this.pendingRefresh; + if (pending && pending !== sharedPass) { + return pending.checksForSnapshotChanges + ? pending.promise + : this.startSnapshotRefreshAfter(pending); + } + return this.startRefreshPass(true); + }); } private runActivationDiscoveryPass(): void { @@ -294,7 +353,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return; } - void this.getOrStartRefreshPass() + void this.getOrStartRefreshPass(true) .then((shouldRetry) => { if (this.disposed || !this.activationDiscoveryActive) { return; @@ -314,7 +373,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { }); } - private async refreshDiscoveredEnvironments(): Promise { + private async refreshDiscoveredEnvironments(checkForSnapshotChanges: boolean): Promise { const cacheRoot = getScriptEnvCacheRoot(this.globalStorageUri); const previousByKey = new Map( this.collection.map((environment) => [this.getDiscoveredEnvironmentKey(environment), environment]), @@ -339,7 +398,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { let shouldRetry = false; for (const entryName of entryNames.sort()) { if (entryName.endsWith('.lock')) { - lockedKeys.add(normalizePath(Uri.joinPath(cacheRoot, entryName.slice(0, -5)).fsPath)); + lockedKeys.add(this.getDiscoveryEntryKey(entryName.slice(0, -5))); shouldRetry = true; continue; } @@ -349,7 +408,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } const envDir = Uri.joinPath(cacheRoot, entryName); - const key = normalizePath(envDir.fsPath); + const key = this.getDiscoveryEntryKey(entryName); const discovered = await this.inspectDiscoveredCacheEntry(cacheRoot, envDir); if (discovered.kind === 'resolved') { nextByKey.set(key, discovered.environment); @@ -371,6 +430,25 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { return false; } + if (checkForSnapshotChanges) { + try { + const finalEntryNames = await fs.readdir(cacheRoot.fsPath); + const initialEntries = new Set(entryNames); + if ( + finalEntryNames.length !== entryNames.length || + finalEntryNames.some((entryName) => !initialEntries.has(entryName)) + ) { + shouldRetry = true; + } + } catch { + shouldRetry = true; + } + } + + if (this.disposed) { + return false; + } + // Preserve previously known entries when a refresh cannot safely classify // them because a build is in progress or the filesystem is transiently unavailable. this.replaceDiscoveredEnvironments(sortEnvironments(Array.from(nextByKey.values()))); @@ -468,8 +546,12 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } } + private getDiscoveryEntryKey(entryName: string): string { + return normalizePath(entryName); + } + private getDiscoveredEnvironmentKey(environment: PythonEnvironment): string { - return normalizePath(environment.sysPrefix); + return this.getDiscoveryEntryKey(path.basename(environment.sysPrefix)); } private isSameDiscoveredEnvironment(first: PythonEnvironment, second: PythonEnvironment): boolean { @@ -1037,10 +1119,15 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable { } private async isCacheEntryBusy(envDirPath: string): Promise { - return ( - this.pendingCreations.has(path.basename(envDirPath)) || - (await fs.pathExists(`${path.resolve(envDirPath)}.lock`)) - ); + if (this.pendingCreations.has(path.basename(envDirPath))) { + return true; + } + try { + await fs.lstat(`${path.resolve(envDirPath)}.lock`); + return true; + } catch (error) { + return !isFileNotFoundError(error); + } } private bumpAssociationRevision(scriptPath: string): void { diff --git a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts index 4daf64d6d..451b44007 100644 --- a/src/test/managers/builtin/inlineScript/envManager.unit.test.ts +++ b/src/test/managers/builtin/inlineScript/envManager.unit.test.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. import assert from 'assert'; +import fsExtra from 'fs-extra'; import * as fs from 'fs-extra'; import * as os from 'os'; import * as path from 'path'; @@ -1564,6 +1565,19 @@ suite('InlineScriptEnvManager', () => { ); }); + test('explicit refresh takes a single cache-root snapshot', async () => { + const environment = await createOwnedEnvironment(); + const sidecar = await makeSidecar(); + setSidecarResults({ [CACHE_KEY]: { kind: 'valid', metadata: sidecar } }); + setResolvedVenvs([environment]); + const readdirStub = sinon.stub(fsExtra, 'readdir').resolves([CACHE_KEY]); + + await manager.refresh(undefined); + + assert.strictEqual(readdirStub.callCount, 1); + assert.deepStrictEqual(await manager.getEnvironments('all'), [environment]); + }); + test('refresh skips missing, invalid, unavailable, and non-directory cache entries', async () => { const valid = await createOwnedEnvironment(); const cacheRoot = cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath; @@ -1607,6 +1621,73 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(listener.callCount, 0); }); + test('refresh preserves a discovered environment when its lock probe reports EIO', async () => { + const environment = await createOwnedEnvironment(); + const sidecar = await makeSidecar(); + setSidecarResults({ [CACHE_KEY]: { kind: 'valid', metadata: sidecar } }); + setResolvedVenvs([environment]); + await manager.refresh(undefined); + resolveVenvStub.resetHistory(); + baseInterpreterStatusStub.resetHistory(); + baseInterpreterStatusStub.resolves('missing'); + const listener = sinon.spy(); + manager.onDidChangeEnvironments(listener); + const lockPath = `${path.resolve(environment.sysPrefix)}.lock`; + sinon + .stub(fsExtra, 'lstat') + .callThrough() + .withArgs(lockPath) + .rejects(Object.assign(new Error('I/O error'), { code: 'EIO' })); + + await manager.refresh(undefined); + + assert.deepStrictEqual(await manager.getEnvironments('all'), [environment]); + assert.strictEqual(resolveVenvStub.callCount, 0); + assert.strictEqual(baseInterpreterStatusStub.callCount, 0); + assert.strictEqual(listener.callCount, 0); + }); + + test('uses the cache entry name to preserve a canonical sysPrefix through a cache-root link', async function () { + const cacheRoot = cacheLayout.getScriptEnvCacheRoot(globalStorageUri).fsPath; + const physicalCacheRoot = path.join(tempRoot, 'physical-cache-root'); + const physicalEnvDir = path.join(physicalCacheRoot, CACHE_KEY); + const physicalExecutable = getVenvPythonPath(physicalEnvDir); + await fs.ensureDir(path.dirname(cacheRoot)); + await fs.ensureDir(physicalCacheRoot); + try { + await fs.symlink(physicalCacheRoot, cacheRoot, isWindows() ? 'junction' : 'dir'); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === 'EPERM' || code === 'EACCES') { + this.skip(); + return; + } + throw error; + } + await fs.outputFile(physicalExecutable, ''); + const environment = makeEnvironment( + 'ms-python.python:inline-script', + '3.12.4', + physicalExecutable, + physicalEnvDir, + ); + const sidecar = await makeSidecar(); + setSidecarResults({ [CACHE_KEY]: { kind: 'valid', metadata: sidecar } }); + resolveVenvStub.resolves(environment); + await manager.refresh(undefined); + resolveVenvStub.resetHistory(); + const listener = sinon.spy(); + manager.onDidChangeEnvironments(listener); + await fs.remove(physicalEnvDir); + await fs.ensureDir(`${physicalEnvDir}.lock`); + + await manager.refresh(undefined); + + assert.deepStrictEqual(await manager.getEnvironments('all'), [environment]); + assert.strictEqual(resolveVenvStub.callCount, 0); + assert.strictEqual(listener.callCount, 0); + }); + test('refresh removes a previously discovered environment when launcher inspection marks it missing', async () => { const environment = await createOwnedEnvironment(); const sidecar = await makeSidecar(); @@ -1652,6 +1733,127 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(listener.callCount, 1); }); + test('runs a snapshot-aware follow-up when activation joins an explicit refresh', async () => { + const first = await createOwnedEnvironment(); + const secondKey = 'fedcba9876543210'; + const sidecar = await makeSidecar(); + setSidecarResults({ [CACHE_KEY]: { kind: 'valid', metadata: sidecar } }); + let second: PythonEnvironment | undefined; + let releaseFirstResolution: (() => void) | undefined; + let signalFirstResolution: (() => void) | undefined; + const firstResolution = new Promise((resolve) => { + signalFirstResolution = resolve; + }); + const resolutionGate = new Promise((resolve) => { + releaseFirstResolution = resolve; + }); + let firstResolutionPending = true; + resolveVenvStub.callsFake(async (candidatePath: string) => { + if ( + firstResolutionPending && + normalizePath(candidatePath) === normalizePath(first.environmentPath.fsPath) + ) { + firstResolutionPending = false; + signalFirstResolution!(); + await resolutionGate; + } + return normalizePath(candidatePath) === normalizePath(first.environmentPath.fsPath) ? first : second; + }); + + const refresh = manager.refresh(undefined); + await firstResolution; + manager.startActivationDiscovery(); + second = await createOwnedEnvironment(secondKey); + setSidecarResults({ + [CACHE_KEY]: { kind: 'valid', metadata: sidecar }, + [secondKey]: { kind: 'valid', metadata: sidecar }, + }); + releaseFirstResolution!(); + await refresh; + + await waitForStubCallCount(resolveVenvStub, 3); + await waitForCondition( + async () => (await manager.getEnvironments('all')).length === 2, + 'Expected activation discovery to scan the entry added after the explicit refresh snapshot', + ); + assert.deepStrictEqual(await manager.getEnvironments('all'), [first, second]); + }); + + test('retries when a cache entry appears during a discovery scan', async () => { + const first = await createOwnedEnvironment(); + const secondKey = 'fedcba9876543210'; + const sidecar = await makeSidecar(); + setSidecarResults({ [CACHE_KEY]: { kind: 'valid', metadata: sidecar } }); + let second: PythonEnvironment | undefined; + let releaseFirstResolution: (() => void) | undefined; + let signalFirstResolution: (() => void) | undefined; + const firstResolution = new Promise((resolve) => { + signalFirstResolution = resolve; + }); + const resolutionGate = new Promise((resolve) => { + releaseFirstResolution = resolve; + }); + let firstResolutionPending = true; + resolveVenvStub.callsFake(async (candidatePath: string) => { + if ( + firstResolutionPending && + normalizePath(candidatePath) === normalizePath(first.environmentPath.fsPath) + ) { + firstResolutionPending = false; + signalFirstResolution!(); + await resolutionGate; + } + return normalizePath(candidatePath) === normalizePath(first.environmentPath.fsPath) ? first : second; + }); + const retryManager = manager as unknown as { + getDiscoveryRetryDelayMs(attempt: number): number | undefined; + }; + sinon.stub(retryManager, 'getDiscoveryRetryDelayMs').callsFake((attempt) => (attempt === 0 ? 0 : undefined)); + + manager.startActivationDiscovery(); + await firstResolution; + second = await createOwnedEnvironment(secondKey); + setSidecarResults({ + [CACHE_KEY]: { kind: 'valid', metadata: sidecar }, + [secondKey]: { kind: 'valid', metadata: sidecar }, + }); + releaseFirstResolution!(); + + await waitForStubCallCount(resolveVenvStub, 3); + await waitForCondition( + async () => (await manager.getEnvironments('all')).length === 2, + 'Expected the scan after the changed snapshot to publish both environments', + ); + assert.deepStrictEqual(await manager.getEnvironments('all'), [first, second]); + }); + + test('discovers a build that completes after the short retry window', async () => { + const sidecar = await makeSidecar(); + setSidecarResults({ [CACHE_KEY]: { kind: 'valid', metadata: sidecar } }); + const retryManager = manager as unknown as { + getDiscoveryRetryDelayMs(attempt: number): number | undefined; + }; + assert.strictEqual(retryManager.getDiscoveryRetryDelayMs(2), 30_000); + const retryDelayStub = sinon + .stub(retryManager, 'getDiscoveryRetryDelayMs') + .callsFake((attempt) => (attempt < 2 ? 0 : attempt === 2 ? 25 : undefined)); + const lockPath = `${path.resolve(envDir().fsPath)}.lock`; + await fs.ensureDir(lockPath); + + manager.startActivationDiscovery(); + await waitForStubCallCount(retryDelayStub, 3); + const environment = await createOwnedEnvironment(); + resolveVenvStub.resolves(environment); + await fs.remove(lockPath); + + await waitForStubCall(resolveVenvStub); + await waitForCondition( + async () => (await manager.getEnvironments('all')).length === 1, + 'Expected the extended retry to publish the completed build', + ); + assert.deepStrictEqual(await manager.getEnvironments('all'), [environment]); + }); + test('explicit refresh does not schedule a delayed follow-up after an uncertain pass', async () => { const sidecar = await makeSidecar(); await createOwnedEnvironment(); @@ -2076,6 +2278,28 @@ suite('InlineScriptEnvManager', () => { assert.strictEqual(listener.callCount, 0); }); + test('preserves a warm association when its lock probe reports EIO', async () => { + const uri = scriptUri(); + const environment = await createOwnedEnvironment(); + await manager.set(uri, environment); + const listener = sinon.spy(); + manager.onDidChangeEnvironment(listener); + await fs.remove(environment.environmentPath.fsPath); + const lockPath = `${path.resolve(environment.sysPrefix)}.lock`; + sinon + .stub(fsExtra, 'lstat') + .callThrough() + .withArgs(lockPath) + .rejects(Object.assign(new Error('I/O error'), { code: 'EIO' })); + clock.tick(5_000); + + assert.strictEqual(await manager.get(uri), undefined); + assert.deepStrictEqual(persistedAssociations, { + [normalizePath(uri.fsPath)]: environment.environmentPath.fsPath, + }); + assert.strictEqual(listener.callCount, 0); + }); + test('refreshes a warm association rebuilt at the same cache path', async () => { const uri = scriptUri(); const environment = await createOwnedEnvironment();