Skip to content

API: Advertise EnvironmentManager and PackageManager capabilities - #1874

Open
Eduardo Villalpando Mello (edvilme) wants to merge 47 commits into
mainfrom
api-2-manager-capabilities
Open

Eduardo Villalpando Mello (edvilme) wants to merge 47 commits into
mainfrom
api-2-manager-capabilities

Conversation

@edvilme

@edvilme Eduardo Villalpando Mello (edvilme) commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Contributes to #1837.

Summary

Adds manager-agnostic feature detection for environment and package managers, with flat capability keys exposed through EnvironmentCapability and PackageCapability.

  • Managers may advertise contextual, read-only checks returning Support: supported, or unsupported with a reason. Support is advisory; unexpected probe failures reject rather than being treated as unsupported.
  • Capability maps are optional for legacy compatibility. Omitted maps use the new method-presence inference helpers and return isLegacy: true. Supplied maps must advertise every catalog key; missing keys return unsupported without legacy inference.
  • Adds public getEnvironmentManagerCapability and getPackageManagerCapability queries. Queries use currently registered providers without activation, registration waits, or prompts. Package queries honor project-bound routing; unregistered explicit projects return unavailable rather than using a containing project's provider.
  • Built-in managers advertise extension-supported behavior, including inline-script creation requirements, Poetry project binding, pip version-lookup support, and the Python 2 restriction on pip installation, uninstallation, and upgrades.
  • Adds capability-query telemetry with the routed manager, tool version, outcome, and legacy flag. Version collection runs asynchronously with a one-second timeout and cannot delay support results or probe errors.
  • Bumps @vscode/python-environments to 1.7.0 and updates public exports, package source copying, API-change checks, and documentation.

Validation

  • Lint and TypeScript checks passed.
  • Extension and API package builds passed; modern and legacy API consumer type-checks passed.
  • 2,883 unit tests passed, with 7 pending.
  • 2 capability integration tests passed, including production nested-project routing coverage.

This comment was marked as resolved.

This comment was marked as resolved.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/capabilities.ts Outdated
Comment thread src/managers/common/registeredManagers.ts
Comment thread src/managers/pipenv/pipenvManager.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Pip version lookup is misreported for unsupported versions, and capability-only API changes bypass release documentation guards.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
Resolved since last review (3)
Files not reviewed (1)
  • api/package-lock.json: Generated file

Comment thread api/scripts/copy-sources.cjs
Comment thread src/managers/builtin/pipPackageManager.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The API fixture does not compile, legacy providers can crash capability lookup, and the breaking package version violates SemVer.

7 open findings
2 resolved since last review
Files not reviewed (1)
  • api/package-lock.json: Generated file

🧠 Review effort: Balanced

Comment thread api/test/consumer.ts
Comment thread api/package.json Outdated
Comment thread src/capabilities.ts Outdated
Comment thread src/managers/builtin/pipPackageManager.ts
Comment thread api/CHANGELOG.md Outdated
Comment thread docs/README.md Outdated
Comment thread src/managers/common/registeredManagers.ts Outdated
Eduardo Villalpando Mello (edvilme) added a commit that referenced this pull request Oct 12, 2026
Contributes to #1837
As part of the efforts to streamline the API and simplify feature
detection (#1874), environment managers should have a mechanism to
advertise their versions. `PackageManager`'s already do this with a
`async getVersion()` method.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Exact URI comparison can incorrectly reject registered projects on Windows, and one capability reason misclassifies system-owned environments.

0 open findings

Files not reviewed (1)
  • api/package-lock.json: Generated file
Previously missed (2)

In code that hasn't changed since last review

Medium severity Normalize file URI paths before project comparison

src/​extensionApi.ts:205

This exact-URI comparison can reject a registered file project on Windows when the caller supplies an equivalent URI with different path casing or separators, returning unavailable before routing. Exact project checks elsewhere normalize fsPath (src/features/envManagers.ts:1070-1075); use the same normalized path comparison here (while preserving scheme/authority as needed).

Low severity Avoid misclassifying SysPythonManager environments as global

src/​managers/​builtin/​sysPythonManager.ts:48

This reason incorrectly assumes every environment owned by SysPythonManager is a global installation. The manager also resolves arbitrary unknown interpreters and registers them under itself (sysPythonManager.ts:295-303, utils.ts:257-273), so a local virtual environment can receive this message. Describe the unsupported manager operation without classifying the environment as global.

🧠 Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Broad public API, routing, and telemetry changes warrant final human validation despite comprehensive coverage.

0 open findings

Files not reviewed (1)
  • api/package-lock.json: Generated file

🧠 Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

File-project routing can reject equivalent registered paths containing trailing separators or dot segments.

1 open finding
Files not reviewed (1)
  • api/package-lock.json: Generated file

🧠 Review effort: Balanced

Comment thread src/extensionApi.ts Outdated
Co-authored-by: edvilme <5952839+edvilme@users.noreply.github.com>
Comment thread src/capabilities.ts
@heejaechang

Copy link
Copy Markdown

Result: ⚠️ needs-more-tests

Verification details

Verification: Isolated verification failed: Consumer fixture type-checks using built declarations and locked dependencies. The relevant tests could not be fully run in the isolated environment; this review is not fully verified.

Summary: Targeted unit and regression runs passed: **442 passing, 5 pending**. TypeScript test compilation and the API package build also passed. However, both consumer fixtures failed type-checking: `api/test/consumer.ts:65` references `PackageCapability.ListAvailableVersions`, while the implementation exports `FetchAvailableVersions`. Full package validation was blocked by an uncached dependency, and Electron integration tests were not run, so the PR&#x27;s validation claims are not fully confirmed.

Test runs: 4 passed, 1 failed, 2 not run

  • ⚠️ Not run | API build and packaged-consumer validation | npm run copy:sources && npm run compile && npm_config_offline=true npm_config_audit=false npm_config_fund=false npm run test:package
  • ⚠️ Not run | Manager capabilities integration | npm run integration-test -- --grep 'Manager capabilities integration'
  • ❌ Failed | PR-related | Consumer fixture type-checks using built declarations and locked dependencies | node <<'NODE'
    const fs = require('node:fs');
    const path = require('node:path');
    const { spawnSync } = require('node:child_process');
    const root = path.resolve('.verification-consumers');
    fs.mkdirSync(root, { recursive: true });
    let failed = false;
    for (const [name, type] of [['modern', 'module'], ['legacy', 'commonjs']]) {
    const dir = path.join(root, name);
    fs.mkdirSync(dir, { recursive: true });
    fs.copyFileSync('test/consumer.ts', path.join(dir, 'consumer.ts'));
    fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ private: true, type }));
    const config = JSON.parse(fs.readFileSync(test/tsconfig.${name}.json, 'utf8'));
    config.compilerOptions.typeRoots = [path.resolve('node_modules/@types')];
    config.compilerOptions.baseUrl = dir;
    config.compilerOptions.paths = { '@vscode/python-environments': [path.resolve('out/types/main.d.ts')] };
    fs.writeFileSync(path.join(dir, 'tsconfig.json'), JSON.stringify(config));
    console.log(Type-check unchanged ${name} consumer against built API declarations using installed locked dependencies);
    const result = spawnSync(process.execPath, ['node_modules/typescript/bin/tsc', '--project', path.join(dir, 'tsconfig.json')], { stdio: 'inherit' });
    console.log(${name} consumer exit=${result.status});
    failed ||= result.status !== 0;
    }
    process.exitCode = failed ? 1 : 0;
    NODE
  • ✅ Passed | Capability unit tests and pip regression tests | npm run compile-tests && node ./node_modules/mocha/bin/mocha.js --no-config --require source-map-support/register --require ./out/test/unittests.js --ui tdd --timeout 180000 --reporter spec out/test/capabilities.unit.test.js out/test/legacyCapabilities.unit.test.js out/test/extensionApi.capabilities.unit.test.js out/test/managers/builtinCapabilities.unit.test.js out/test/managers/builtin/pipPackageManager.unit.test.js
  • ✅ Passed | API, routing, scoped-manager, and built-in manager regressions | node ./node_modules/mocha/bin/mocha.js --no-config --require source-map-support/register --require ./out/test/unittests.js --ui tdd --timeout 180000 --reporter dot out/test/extensionApi.unit.test.js out/test/features/envManagers.unit.test.js out/test/managers/common/projectScopedPackageManagerCache.unit.test.js out/test/managers/common/registeredManagers.versionLookup.unit.test.js out/test/managers/conda/condaPackageManager.unit.test.js out/test/managers/poetry/poetryPackageManager.unit.test.js out/test/managers/builtin/inlineScript/envManager.unit.test.js
  • ✅ Passed | Dependency and test discovery | printf 'Sandbox profile: %s\n' "$AUTOMATION_SANDBOX_PROFILE"; node --version; npm --version; git status --short; git log -1 --format='%h %s'; node -e "const fs=require('fs'); for (const p of ['package.json','api/package.json']) { const j=require('./'+p); console.log(p, JSON.stringify(j.scripts,null,2)); } for(const p of ['node_modules','api/node_modules','out/test','build/.mocha.unittests.json','build/ci/unittest.js']) console.log(p,fs.existsSync(p));"; git diff HEAD^ HEAD --stat; find build src/test api/test -maxdepth 3 -type f ( -iname 'mocha' -o -iname 'capabilit' -o -name 'unittests.ts' ) -print
  • ✅ Passed | Offline locked dependency preparation | npm ci --offline --ignore-scripts --no-audit --no-fund && npm ci --offline --ignore-scripts --no-audit --no-fund --prefix api
⚠️ API build and packaged-consumer validation diagnostic output
Source copying and declaration, ESM, and CommonJS compilation completed successfully.
Package validation stopped during dependency preparation:
npm error code ENOTCACHED
npm error request to https://packagefeedproxy.microsoft.io/npm/@types/vscode/-/vscode-1.140.0.tgz failed: cache mode is 'only-if-cached' but no cached response is available.
⚠️ Manager capabilities integration diagnostic output
Requires an Electron/VS Code extension host and downloaded extensions; unsupported by this offline verification environment.
❌ Consumer fixture type-checks using built declarations and locked dependencies diagnostic output
Both modern and legacy consumers failed with:
consumer.ts(59,7): error TS2741: Property '"packages.fetchversions"' is missing ... but required in type 'Readonly<Record<PackageManagerCapability, CapabilityCheck>>'.
consumer.ts(65,24): error TS2551: Property 'ListAvailableVersions' does not exist ... Did you mean 'FetchAvailableVersions'?
modern consumer exit=2
legacy consumer exit=2

@heejaechang Heejae Chang (heejaechang) added the review-auto:changes-requested Automated review: posted blocking findings to address. label Oct 14, 2026
@heejaechang

Copy link
Copy Markdown

Result: ⚠️ partially-verified

Verification details

Verification: The relevant tests could not be fully run in the isolated environment; this review is not fully verified.

Summary: Targeted capability tests passed: 41 tests covering legacy inference, provider probes, routing, telemetry timeouts, and built-in restrictions. Existing API, routing, and manager regression tests also passed. TypeScript compilation, lint, the extension build, and API package compilation succeeded. Packaged consumer validation was blocked by an uncached offline dependency; the two new Electron integration tests were not run. Verification is therefore partial.

Test runs: 5 passed, 2 not run

  • ⚠️ Not run | API build and packaged consumer validation | npm run copy:sources && npm run compile && npm_config_offline=true npm_config_audit=false npm_config_fund=false npm run test:package
  • ⚠️ Not run | Manager capabilities Electron integration tests
  • ✅ Passed | Capability tests and pip regressions | npm run compile-tests && node ./node_modules/mocha/bin/mocha.js --no-config --ui tdd --require source-map-support/register --require ./out/test/unittests.js --timeout 30000 out/test/capabilities.unit.test.js out/test/legacyCapabilities.unit.test.js out/test/extensionApi.capabilities.unit.test.js out/test/managers/builtinCapabilities.unit.test.js out/test/managers/builtin/pipPackageManager.unit.test.js
  • ✅ Passed | API routing and manager regressions | node ./node_modules/mocha/bin/mocha.js --no-config --ui tdd --require source-map-support/register --require ./out/test/unittests.js --timeout 30000 out/test/extensionApi.unit.test.js out/test/features/envManagers.unit.test.js out/test/features/packageManager.api.unit.test.js out/test/managers/common/projectScopedPackageManagerCache.unit.test.js out/test/managers/common/registeredManagers.versionLookup.unit.test.js out/test/managers/poetry/poetryPackageManager.unit.test.js out/test/managers/conda/condaPackageManager.unit.test.js out/test/managers/builtin/inlineScript/envManager.unit.test.js
  • ✅ Passed | Dependency and test discovery | pwd; printf '\nSandbox profile: '; printenv AUTOMATION_SANDBOX_PROFILE; printf '\nGit state\n'; git status --short; printf '\nRoot scripts and dependencies\n'; node -e 'const p=require("./package.json");console.log(JSON.stringify({scripts:p.scripts,devDependencies:p.devDependencies},null,2))'; printf '\nAPI scripts\n'; node -e 'console.log(JSON.stringify(require("./api/package.json"),null,2))'; printf '\nDependency and runner availability\n'; for p in node_modules node_modules/mocha/bin/mocha.js node_modules/typescript/bin/tsc api/node_modules .mocharc.json .mocharc.js; do if test -e "$p"; then ls -ld "$p"; fi; done; printf '\nTest entrypoints\n'; git ls-files 'mocha' 'unit' 'testconfig*' 'testsetup*' 'api/test/*' | head -90
  • ✅ Passed | Offline dependency preparation | npm ci --offline --ignore-scripts --no-audit --no-fund && npm --prefix api ci --offline --ignore-scripts --no-audit --no-fund
  • ✅ Passed | Lint and extension build | npm run lint > /tmp/capability-lint.log 2>&1; lint_status=$?; tail -n 20 /tmp/capability-lint.log; printf '\nLint exit: %s\n' "$lint_status"; npm run compile > /tmp/capability-build.log 2>&1; build_status=$?; tail -n 30 /tmp/capability-build.log; printf '\nExtension build exit: %s\n' "$build_status"; if test "$lint_status" -ne 0; then exit "$lint_status"; fi; exit "$build_status"
⚠️ API build and packaged consumer validation diagnostic output
Source copying and declaration, ESM, and CommonJS compilation succeeded.
Packaged consumer tests could not start:
npm error code ENOTCACHED
npm error request to https://packagefeedproxy.microsoft.io/npm/@types/vscode/-/vscode-1.140.0.tgz failed: cache mode is 'only-if-cached' but no cached response is available.
[container exit=1]
⚠️ Manager capabilities Electron integration tests diagnostic output
Two new integration tests require a VS Code Electron extension host and the Python extension. Electron tests requiring downloaded VS Code are unsupported in this verification environment; no integration command was invoked.

@heejaechang Heejae Chang (heejaechang) added review-auto:approved Automated review: no blocking findings (approval posted). and removed review-auto:changes-requested Automated review: posted blocking findings to address. labels Oct 14, 2026
@heejaechang

Copy link
Copy Markdown

Result: ⚠️ partially-verified

Verification details

Verification: Isolated verification observed failures that were not classified as caused by this PR: Dependency and runner discovery. The relevant tests could not be fully run in the isolated environment; this review is not fully verified.

Summary: Ran 63 targeted unit tests in the disposable container; all passed, covering capability resolution, legacy inference, routing, telemetry timeouts, built-in managers, and existing API behavior. TypeScript compilation, extension and API builds, and linting of changed TypeScript files passed. Packaged-consumer validation was blocked by an uncached offline dependency, and the two new integration tests were not run because they require a VS Code extension host. Verification is therefore partial; no executed test failed.

Test runs: 4 passed, 1 failed, 2 not run

  • ⚠️ Not run | API build and packaged-consumer validation | npm run copy:sources && npm run compile && npm_config_offline=true npm_config_audit=false npm_config_fund=false npm run test:package
  • ⚠️ Not run | Manager capabilities integration tests
  • ❌ Failed | unrelated to this PR | Dependency and runner discovery | printf 'PROFILE=%s\n' "$AUTOMATION_SANDBOX_PROFILE"; node --version; npm --version; git diff --stat; node -e "const p=require('./package.json'); console.log(JSON.stringify({scripts:p.scripts,devDependencies:p.devDependencies},null,2)); const fs=require('fs'); for (const x of ['node_modules','node_modules/mocha/bin/mocha.js','node_modules/typescript/bin/tsc','api/node_modules','out/test','build/testSettings.js']) console.log(x,fs.existsSync(x));"; ls build; ls -a | grep mocha
  • ✅ Passed | Capability and pip unit tests | npm run compile-tests && node ./node_modules/mocha/bin/mocha.js --no-config --ui tdd --require source-map-support/register --require ./out/test/unittests.js --timeout 30000 out/test/capabilities.unit.test.js out/test/legacyCapabilities.unit.test.js out/test/extensionApi.capabilities.unit.test.js out/test/managers/builtinCapabilities.unit.test.js out/test/managers/builtin/pipPackageManager.unit.test.js
  • ✅ Passed | Existing API, manager, and version-lookup regressions | node ./node_modules/mocha/bin/mocha.js --no-config --ui tdd --require source-map-support/register --require ./out/test/unittests.js --timeout 30000 out/test/extensionApi.unit.test.js out/test/features/envManagers.unit.test.js out/test/managers/common/registeredManagers.versionLookup.unit.test.js
  • ✅ Passed | Offline dependency preparation | npm ci --offline --ignore-scripts --no-audit --no-fund && npm ci --offline --ignore-scripts --no-audit --no-fund --prefix api
  • ✅ Passed | Extension build and changed-TypeScript lint | npm run compile && node ./node_modules/eslint/bin/eslint.js --config=eslint.config.mjs src/api.ts src/capabilities.ts src/legacyCapabilities.ts src/extensionApi.ts src/types.ts src/common/telemetry/constants.ts src/managers/builtin/inlineScript/envManager.ts src/managers/builtin/pipPackageManager.ts src/managers/builtin/sysPythonManager.ts src/managers/builtin/venvManager.ts src/managers/common/registeredManagers.ts src/managers/conda/condaEnvManager.ts src/managers/conda/condaPackageManager.ts src/managers/pipenv/pipenvManager.ts src/managers/poetry/poetryManager.ts src/managers/poetry/poetryPackageManager.ts src/managers/pyenv/pyenvManager.ts src/test/capabilities.unit.test.ts src/test/capabilityFixtures.ts src/test/extensionApi.capabilities.unit.test.ts src/test/integration/managerCapabilities.integration.test.ts src/test/legacyCapabilities.unit.test.ts src/test/managers/builtin/pipPackageManager.unit.test.ts src/test/managers/builtinCapabilities.unit.test.ts
⚠️ API build and packaged-consumer validation diagnostic output
Source copying and declaration, ESM, and CommonJS builds passed. Consumer validation stopped during dependency installation, before type-checking or runtime assertions:
npm error code ENOTCACHED
npm error request to https://packagefeedproxy.microsoft.io/npm/@types/vscode/-/vscode-1.140.0.tgz failed: cache mode is 'only-if-cached' but no cached response is available.
⚠️ Manager capabilities integration tests diagnostic output
Not attempted: these two tests require a VS Code extension host and the Python extension. Electron tests requiring a downloaded VS Code instance are unsupported in this verification environment.
❌ Dependency and runner discovery diagnostic output
PROFILE=typescript
v22.21.1
10.9.4
warning: Not a git repository.
node_modules false
node_modules/mocha/bin/mocha.js false
node_modules/typescript/bin/tsc false
api/node_modules false
out/test false
The container lacks Git metadata; the final grep also found no root-level Mocha filename. Manifest and dependency discovery completed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved via Review Center.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

debt Code quality issues no-merge PR is currently blocked from merging review-auto:approved Automated review: no blocking findings (approval posted).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants