fix: preserve out-of-scope environments during venv scoped refresh by StellaHuang95 · Pull Request #15 · StellaHuang95/vscode-python-environments

StellaHuang95 · 2026-08-22T02:52:43Z

Problem

VenvManager supports URI-scoped refreshes (refresh(scope)), triggered for example after creating an environment in a specific workspace folder (envCommands calls m.refresh(options?.rootUri)) or via api.refreshEnvironments(uri). A scoped discovery is authoritative only for the target scope, but the manager treated its results as authoritative for the entire collection.

Reproduction (multi-root workspace)

Open a multi-root workspace with folders A and B, each containing its own .venv, plus a global env from python.venvFolders / ~/.virtualenvs.
All three environments are discovered and listed.
Trigger a scoped refresh of folder A (create/select an env in A, or api.refreshEnvironments(Uri.file(A))).
Before this fix: folder B's environment and the global env vanish from the list and onDidChangeEnvironments fires spurious remove events for them — even though only A was refreshed.

Root cause

internalRefresh ran findVirtualEnvironments(..., [scope]) — which searches only the scope plus configured venvFolders, skipping global discovery — and then replaced this.collection with just those scoped results, firing remove for every previously-known environment regardless of whether it was in the scope.

Fix

The scoped branch now merges into the collection instead of replacing it (full refresh and all other managers are unchanged):

Scope directory. A scope may be a file, so mergeScopedEnvironments resolves it to a directory via the existing findParentIfFile helper (falling back to the raw scope if the path can't be inspected).
Partition. The current collection is split into the envs inside the scope and those outside it using a new isPathInside(scope, candidate) helper (path.resolve + path.relative, inclusive of the scope, sibling-prefix safe so .../app ⊄ .../app-2).
Retain every env outside the scope (siblings, globals, venvFolders) untouched — same objects, no events.
Replace the in-scope region with the freshly discovered envs, dropping any discovered result whose normalized path duplicates a retained env (so a global venvFolders env that scoped discovery also returns isn't duplicated).
Events cover only in-scope changes: remove for the previous in-scope envs and add for the newly present ones. Retained siblings/globals never surface.

loadEnvMap() and the full remove-all / add-all behavior of an unscoped refresh are kept as-is.

Tests

New venvManager.scopedRefresh.unit.test.ts (hermetic, multi-root; topology app vs app-2, global outside root):

sibling + global retention while the in-scope env is rediscovered (only in-scope remove/add fire),
target-only stale removal, new in-scope add,
a venvFolders duplicate outside the scope from discovery is ignored (existing global object retained, not churned),
sibling-prefix safety (app vs app-2),
unchanged full-refresh behavior, scope pass-through to discovery, and file-scope → directory resolution (plus the inspect-failure fallback).

pathUtils.unit.test.ts adds an isPathInside suite (equal/child/nested/parent/sibling-prefix/unrelated/relative + Windows drive and case-folding cases).

npm run lint, npm run compile-tests, and npm run unittest (1709 passing) are green.

Fixes the scoped-refresh regression for multi-root workspaces.

StellaHuang95 · 2026-08-22T04:24:36Z

🔒 Automated review in progress — @StellaHuang95 is auto-reviewing this PR.

Reconcile only the owning project's persisted map entry (no global loadEnvMap rebuild), serialize refresh transactions, preserve the prior envId on same-path rediscovery, and exclude configured venvFolders roots from authoritative removal. Events are computed from the scope-local transaction. Full (unscoped) refresh and every other manager remain unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

StellaHuang95 · 2026-08-22T07:02:42Z

+            if (refreshedGlobal) {
+                this.globalEnv = refreshedGlobal;
+            }
+        }


Issue · Please address or respond

When an in-scope environment referenced by globalEnv is removed rather than rebuilt, this intentionally retains the deleted object and get(undefined) continues returning it. Clear or safely re-resolve that alias when its environment is removed.

StellaHuang95 · 2026-08-22T07:02:44Z

+    return (
+        relative === '' ||
+        (relative !== '..' && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative))
+    );


Issue · Please address or respond

This duplicates PythonProjectManagerImpl.isUriMatching, and the implementations already disagree for filesystem-root projects. Consolidate project ownership on the shared containment helper so scoped-refresh ownership matches the project manager.

A URI-scoped venv refresh replaced the entire environment collection with only the scoped discovery results, so in a multi-root workspace refreshing one folder removed sibling and global environments and fired spurious removal events. The scoped branch now partitions the collection by path containment, retains out-of-scope environments untouched, replaces only the in-scope region with freshly discovered envs (deduplicated against retained paths), and emits events for in-scope changes only. Full refresh and all other managers are unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

StellaHuang95 · 2026-08-23T02:25:52Z

+        discovered: PythonEnvironment[],
+    ): Promise<DidChangeEnvironmentsEventArgs> {
+        let scopeDir: string;
+        try {


Issue · Please address or respond

findParentIfFile makes a nested file scope such as project/src/main.py merge at project/src. The existing project/.venv is therefore retained, and its rediscovered replacement is discarded by the retained-path filter. Resolve the owning project for the scope before discovery and merging, and add a nested-subdirectory regression test.

[verified]

StellaHuang95 · 2026-08-23T02:25:54Z

+        (relative !== '..' && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative))
+    );
+}
+


Issue · Please address or respond

This duplicates PythonProjectManagerImpl.isUriMatching with conflicting filesystem-root semantics. Consolidate project ownership and scoped-refresh containment on one shared implementation so these rules cannot drift.

[verified]

StellaHuang95 · 2026-08-23T02:25:56Z

+                .filter((env) => !retainedIds.has(env.envId.id))
+                .map((env) => ({ environment: env, kind: EnvironmentChangeKind.add })),
+        ];
+    }


Issue · Please address or respond

loadEnvMap() clears the complete project map before reading each previousEnv, so refreshing workspace A can report persisted selections in unrelated workspace B as newly selected. Preserve the previous map for comparison, or reconcile only the scoped project during a scoped refresh.

[verified]

StellaHuang95 · 2026-08-23T02:26:00Z

GitHub cannot anchor PR review comments to unchanged lines in the diff. Falling back to a general PR comment for src/managers/builtin/venvManager.ts:L408.

Warning · Non-blocking recommendation

Overlapping scoped refreshes can mutate this.collection while another refresh awaits loadEnvMap(), after which event construction reads the other refresh's collection. Serialize the discovery/merge/event transaction or construct events from an immutable next collection.

[verified]

StellaHuang95 · 2026-08-23T02:26:02Z

+
    async getEnvironments(scope: GetEnvironmentsScope): Promise<PythonEnvironment[]> {
        await this.initialize();



Warning · Non-blocking recommendation

Path-only partitioning treats configured global environments physically beneath the project scope, including every environment for a filesystem-root workspace, as authoritative in-scope data. Exclude configured global roots from the replaceable partition or classify discovery results by ownership/provenance.

[verified]

StellaHuang95 · 2026-08-23T02:52:20Z

+        } catch {
+            scopeDir = scope.fsPath;
+        }
+        const inScope = (env: PythonEnvironment): boolean => isPathInside(scopeDir, env.environmentPath.fsPath);


Issue · Please address or respond

For a nested file scope such as project/src/main.py, findParentIfFile produces project/src, so the existing project/.venv is retained and the rediscovered replacement is filtered out by path. Resolve the owning Python project URI before partitioning, and cover a file below the project root with a regression test.

StellaHuang95 · 2026-08-23T02:52:24Z

GitHub cannot anchor PR review comments to unchanged lines in the diff. Falling back to a general PR comment for src/common/utils/pathUtils.ts:L86.

Issue · Please address or respond

isPathInside duplicates PythonProjectManagerImpl.isUriMatching with different filesystem-root semantics. Consolidate project ownership and scoped-refresh containment on one shared implementation to avoid the two paths drifting.

StellaHuang95 · 2026-08-23T02:52:26Z

+                .filter((env) => !retainedIds.has(env.envId.id))
+                .map((env) => ({ environment: env, kind: EnvironmentChangeKind.add })),
+        ];
+    }


Issue · Please address or respond

Calling the collection-wide loadEnvMap() during a scoped refresh reprocesses retained environments from unrelated projects, which can re-report their persisted selections. Preserve the existing map entries or reconcile only the refreshed project.

StellaHuang95 · 2026-08-23T02:52:29Z

+            ...this.collection
+                .filter((env) => !retainedIds.has(env.envId.id))
+                .map((env) => ({ environment: env, kind: EnvironmentChangeKind.add })),
+        ];


Issue · Please address or respond

The merge only deduplicates discovered paths against retained environments; it does not exclude newly discovered out-of-scope venvFolders results. A scoped refresh can therefore add and emit an event for an uncached global environment. Filter discovery results to the authoritative scope, or classify them by project ownership/provenance before merging.

StellaHuang95 · 2026-08-23T02:52:31Z

+
+        const collection: PythonEnvironment[] = (manager as any).collection;
+        assert.ok(collection.includes(envB) && collection.includes(envGlobal));
+        assert.ok(collection.includes(envANew) && !collection.includes(envAOld));


Issue · Please address or respond

These .includes() assertions can pass when an unexpected environment is also present. Assert the exact collection IDs first, then retain identity assertions for the preserved objects.

StellaHuang95 commented Aug 22, 2026

View reviewed changes

Comment thread src/managers/builtin/venvManager.ts Outdated

StellaHuang95 commented Aug 22, 2026

View reviewed changes

Comment thread src/managers/builtin/venvManager.ts Outdated

StellaHuang95 commented Aug 22, 2026

View reviewed changes

Comment thread src/common/utils/pathUtils.ts

StellaHuang95 added the review-auto:changes-requested Automated review: posted blocking findings to address. label Aug 22, 2026

StellaHuang95 added the bug Something isn't working label Aug 22, 2026

StellaHuang95 changed the title ~~fix: preserve out-of-scope environments during venv scoped refresh~~ fix: make scoped venv refresh scope-local and non-destructive Aug 22, 2026

StellaHuang95 commented Aug 22, 2026

View reviewed changes

StellaHuang95 removed the review-auto:changes-requested Automated review: posted blocking findings to address. label Aug 23, 2026

StellaHuang95 force-pushed the preserve-scoped-refresh-results branch from 14d2154 to d33c589 Compare August 23, 2026 02:00

StellaHuang95 changed the title ~~fix: make scoped venv refresh scope-local and non-destructive~~ fix: preserve out-of-scope environments during venv scoped refresh Aug 23, 2026

StellaHuang95 force-pushed the preserve-scoped-refresh-results branch from d33c589 to 87f7b24 Compare August 23, 2026 02:11

StellaHuang95 force-pushed the preserve-scoped-refresh-results branch from 87f7b24 to 6cc58b0 Compare August 23, 2026 02:18

StellaHuang95 commented Aug 23, 2026

View reviewed changes

StellaHuang95 added the review-auto:changes-requested Automated review: posted blocking findings to address. label Aug 23, 2026

StellaHuang95 commented Aug 23, 2026

View reviewed changes

StellaHuang95 closed this Aug 23, 2026


		async getEnvironments(scope: GetEnvironmentsScope): Promise<PythonEnvironment[]> {
		await this.initialize();

Conversation

StellaHuang95 commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Reproduction (multi-root workspace)

Root cause

Fix

Tests

Uh oh!

StellaHuang95 commented Aug 22, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StellaHuang95 commented Aug 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StellaHuang95 commented Aug 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

StellaHuang95 commented Aug 22, 2026 •

edited

Loading