Skip to content

Split UV/Pip PackageManagers - #1941

Merged
Eduardo Villalpando Mello (edvilme) merged 9 commits into
mainfrom
uv-managers
Oct 22, 2026
Merged

Eduardo Villalpando Mello (edvilme) merged 9 commits into
mainfrom
uv-managers

Conversation

@edvilme

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

Copy link
Copy Markdown
Contributor

This pull request introduces a new architecture for Python package management in the extension, adding support for uv as a first-class package manager alongside pip. It refactors the internal routing so that package operations can be dynamically directed to either pip or uv based on user settings and environment context, and ensures that each manager only executes its own commands. The documentation is updated to describe the new behavior and configuration options.

New package manager architecture and routing:

  • Introduces a new BuiltInPackageManager class that routes package operations between pip and uv managers according to user configuration and environment, using the existing automatic selection rules. This enables seamless switching and explicit selection of uv for package operations.
  • Refactors the registration of package managers so that both pip and uv managers are registered, and the new router is used as the default package manager.

Refactoring and code simplification:

  • Refactors PipPackageManager to only manage packages using pip, removing all logic and command creation related to uv. The manager now delegates ownership and events to the router when wrapped. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]

Documentation updates:

  • Updates the README.md to document uv as a supported package manager, describe how to select it, and clarify the routing and fallback behavior for package operations.

Internal consistency and event handling:

  • Ensures that package change events and package item ownership are correctly attributed to the router when PipPackageManager is wrapped, improving downstream event handling and UI consistency. [1] [2] [3]

Code documentation and clarity:

  • Adds and improves docstrings and comments throughout the new and updated classes for better maintainability and clarity. [1] [2] [3]

These changes lay the groundwork for robust, user-configurable Python package management in the extension, with clear separation between pip and uv and improved maintainability.

@edvilme

Copy link
Copy Markdown
Contributor Author

Copilot resolve the merge conflicts in this pull request

Copilot AI commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Copilot resolve the merge conflicts in this pull request

Merged main and resolved the conflicts in 1930418.

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

Cache consistency, explicit uv routing, scoped executable discovery, and test reliability remain unresolved.

5 open issues
What changed in this PR

Separates built-in uv and pip package management and extracts environment-creation backends while preserving the existing venv identity.

Changes:

  • Registers a standalone uv package manager with automatic routing through the existing pip entry point.
  • Separates uv and standard-library environment creation.
  • Adds routing and compatibility tests and documents manager selection.
File Description
src/​test/​managers/​fastPath.get.unit.test.ts Uses the renamed environment manager.
src/​test/​managers/​common/​packageManagerHeadlessConformance.unit.test.ts Updates manager fixture types.
src/​test/​managers/​builtin/​venvUtils.createWithProgress.unit.test.ts Tests backend delegation and ownership.
src/​test/​managers/​builtin/​venvManager.selectionRace.unit.test.ts Updates selection-race fixtures.
src/​test/​managers/​builtin/​venvManager.selection.unit.test.ts Updates selection-recovery fixtures.
src/​test/​managers/​builtin/​venvManager.resolve.unit.test.ts Updates path-resolution fixtures.
src/​test/​managers/​builtin/​venvManager.createRemove.unit.test.ts Updates creation/removal fixtures.
src/​test/​managers/​builtin/​venvManager.autoSelect.unit.test.ts Updates automatic-selection fixtures.
src/​test/​managers/​builtin/​uvPackageManager.unit.test.ts Adds standalone uv tests.
src/​test/​managers/​builtin/​pipPackageManagerCompatibility.unit.test.ts Checks backend command compatibility.
src/​test/​managers/​builtin/​pipPackageManager.unit.test.ts Verifies pip-only execution.
src/​test/​managers/​builtin/​main.unit.test.ts Updates registration/watcher fixtures.
src/​test/​managers/​builtin/​environmentCreation.unit.test.ts Tests creation backends and cancellation.
src/​test/​managers/​builtin/​builtInPackageManager.unit.test.ts Tests package routing and ownership.
src/​test/​internal/​pythonTools.startup.unit.test.ts Updates startup manager fixtures.
src/​test/​internal/​pythonToolManagers.unit.test.ts Updates tool manager fixtures.
src/​test/​internal/​publicApiCompatibility.unit.test.ts Tests routed uv execution policy.
src/​test/​internal/​discoveryIsolation.unit.test.ts Updates discovery-isolation fixtures.
src/​test/​integration/​packageManager.integration.test.ts Adds a uv lifecycle profile.
src/​managers/​builtin/​venvUtils.ts Delegates creation to selected backends.
src/​managers/​builtin/​venvStepBasedFlow.ts Passes backend selection through creation flows.
src/​managers/​builtin/​venvEnvironmentManager.ts Implements standard-library creation.
src/​managers/​builtin/​uvPackageManager.ts Implements standalone uv package operations.
src/​managers/​builtin/​uvEnvironmentManager.ts Implements uv environment creation.
src/​managers/​builtin/​pipUtils.ts Allows backend-specific package listing.
src/​managers/​builtin/​pipPackageManager.ts Restricts execution to pip and supports delegated ownership.
src/​managers/​builtin/​main.ts Registers the router and uv manager.
src/​managers/​builtin/​environmentCreation.ts Defines creation contracts and selection.
src/​managers/​builtin/​builtInPackageManager.ts Routes operations between pip and uv.
src/​managers/​builtin/​builtInEnvironmentManager.ts Preserves venv behavior with injected backends.
README.md Documents backend behavior and uv selection.

Review effort: Balanced

Comment thread src/managers/builtin/builtInPackageManager.ts
Comment thread src/managers/builtin/main.ts
Comment thread src/managers/builtin/uvPackageManager.ts
Comment thread src/test/integration/packageManager.integration.test.ts
Comment thread src/test/managers/builtin/builtInPackageManager.unit.test.ts Outdated
@bschnurr

Bill Schnurr (bschnurr) commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

🔒 Automated review in progress — Bill Schnurr (@bschnurr) is auto-reviewing this PR.

@bschnurr

Copy link
Copy Markdown
Member

Result: 🔴 could-not-verify

Verification details

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

Summary: [unavailable] Container verification could not start and local execution was not authorized for this PR HEAD: C:\Program Files\RedHat\Podman\podman.EXE --connection pyrx-automation failed: stderr: Error: unable to start container "a1bfb53697ab8af7c83614c168c93bf36036adb09bdacf0cffeb0cddec0e3407": crun: open `memory.max` for writing: No such file or directory: OCI runtime attempted to invoke a command that was not found

Test runs: none recorded.

@bschnurr Bill Schnurr (bschnurr) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved via Review Center.

@bschnurr Bill Schnurr (bschnurr) added the review-auto:approved Automated review: no blocking findings (approval posted). label Oct 21, 2026

@rchiodo Rich Chiodo (rchiodo) 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.

Approved via Review Center.

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.

Comment thread src/managers/builtin/builtInPackageManager.ts Outdated
@bschnurr

Copy link
Copy Markdown
Member

Result: 🔴 could-not-verify

Verification details

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

Summary: [unavailable] Container verification could not start and local execution was not authorized for this PR HEAD: C:\Program Files\RedHat\Podman\podman.EXE --connection pyrx-automation failed: stderr: Error: unable to start container "91ee3e0dfd4b65676ccd391fe56544598e75b5b7cd51ba45c720036129b900b2": crun: open `memory.max` for writing: No such file or directory: OCI runtime attempted to invoke a command that was not found

Test runs: none recorded.

@bschnurr Bill Schnurr (bschnurr) added review-auto:changes-requested Automated review: posted blocking findings to address. and removed review-auto:approved Automated review: no blocking findings (approval posted). labels Oct 21, 2026
@bschnurr

Copy link
Copy Markdown
Member

Result: 🔴 could-not-verify

Verification details

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

Summary: [unavailable] Container verification could not start and local execution was not authorized for this PR HEAD: C:\Program Files\RedHat\Podman\podman.EXE --connection pyrx-automation failed: stderr: Error: unable to start container "0a32537dc51660076a1b913108f0138f881de1264b686f1bb073c080546c317d": crun: open `memory.max` for writing: No such file or directory: OCI runtime attempted to invoke a command that was not found

Test runs: none recorded.

@bschnurr Bill Schnurr (bschnurr) 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 21, 2026

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

Both backends can publish stale package-change notifications after concurrent mutations invalidate a pending refresh.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Medium severity Suppress obsolete package events after uv mutations

src/​managers/​builtin/​pipPackageManager.ts:300

If uv mutates this environment while a pip refresh is pending, this callback can still publish changes from the obsolete inventory. updatePackagesAndNotify emits the notification before cachePackages rejects the stale generation, so an initial refresh can report a package as newly added after uv has uninstalled it. Check the captured cacheVersion before firing the event, and cover notification suppression in the concurrent-refresh test as well as cache invalidation.

Medium severity Suppress stale package events after pip mutations

src/​managers/​builtin/​uvPackageManager.ts:263

If pip mutates this environment while a uv refresh is pending, the refresh can still publish changes from its pre-mutation snapshot. updatePackagesAndNotify invokes this callback before cachePackages checks the captured generation. For example, an initial refresh can emit an add event for a package that pip has already uninstalled, even though the stale cache write is rejected. Guard this notification with cacheVersion too, and extend the concurrent-refresh test to assert that superseded refreshes emit no package-change events.

3 resolved since last review

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

Cache coordination can suppress change notifications, and partial mutation failures discard baselines needed to report removals on recovery.

1 open issue
2 previously missed issues

In code that hasn't changed since last review

Medium severity Preserve package baseline after partial uninstall failure

src/​managers/​builtin/​pipPackageManager.ts:236

If an uninstall succeeds but the following install fails or is cancelled, these lines discard both backends' comparison baselines before updatePackagesAndNotify() runs. A subsequent refresh() compares against an empty snapshot, reporting surviving packages as additions and missing the successful removal. Preserve previousPackages as the baseline and pass it to the peer-invalidation event, while still evicting the readable inventory caches. Add a recovery-refresh assertion to the partial-failure tests.

Medium severity Notify package removal after uv partial failure

src/​managers/​builtin/​uvPackageManager.ts:213

If uv successfully uninstalls a package and the following install fails or is cancelled, these lines discard both backends' comparison baselines and execution skips the change notification. A later refresh() cannot report the removal; if the environment is now empty, it emits no event at all. Preserve previousPackages as the baseline and include it in the peer-invalidation event, without restoring it as readable cached inventory. Add a recovery-refresh assertion to the partial-failure tests.

Review effort: Balanced

Comment thread src/managers/builtin/builtInPackageManager.ts
Comment thread src/managers/builtin/uvPackageManager.ts
@bschnurr

Copy link
Copy Markdown
Member

Result: 🔴 could-not-verify

Verification details

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

Summary: [unavailable] Container verification could not start and local execution was not authorized for this PR HEAD: C:\Program Files\RedHat\Podman\podman.EXE --connection pyrx-automation failed: stderr: Error: unable to start container "e7148f613e0f14bfd0b6faaedf1297c47eefea8ec54c807bda7c0a27db417a91": crun: open `memory.max` for writing: No such file or directory: OCI runtime attempted to invoke a command that was not found

Test runs: none recorded.

@bschnurr

Copy link
Copy Markdown
Member

Result: 🔴 could-not-verify

Verification details

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

Summary: [unavailable] Container verification could not start and local execution was not authorized for this PR HEAD: C:\Program Files\RedHat\Podman\podman.EXE --connection pyrx-automation failed: stderr: Error: unable to start container "d2ccca7bd627be1b45d60c40935a08dadba8ddd04294b1d508ba1036b0d4172e": crun: open `memory.max` for writing: No such file or directory: OCI runtime attempted to invoke a command that was not found

Test runs: none recorded.

@edvilme
Eduardo Villalpando Mello (edvilme) merged commit ee1499b into main Oct 22, 2026
84 of 108 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

debt Code quality issues review-auto:approved Automated review: no blocking findings (approval posted).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants