From 4a24437ed9d9bb741e3215608876b0cd9721a2b4 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Sat, 22 Aug 2026 18:59:43 -0700 Subject: [PATCH 1/9] fix: preserve out-of-scope environments during venv scoped refresh 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 resolves the owning project directory for the scope, partitions the collection by path containment, retains out-of-scope environments untouched, admits only freshly discovered in-scope environments (deduplicated against retained paths), and computes the change payload locally before reloading the env map so overlapping refreshes cannot misattribute additions. Full refresh and all other managers are unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/common/utils/pathUtils.ts | 14 + src/managers/builtin/venvManager.ts | 110 +++- src/test/common/pathUtils.unit.test.ts | 51 +- .../venvManager.scopedRefresh.unit.test.ts | 548 ++++++++++++++++++ 4 files changed, 704 insertions(+), 19 deletions(-) create mode 100644 src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts diff --git a/src/common/utils/pathUtils.ts b/src/common/utils/pathUtils.ts index d398828a1..d38efda7d 100644 --- a/src/common/utils/pathUtils.ts +++ b/src/common/utils/pathUtils.ts @@ -65,6 +65,20 @@ export function normalizePath(fsPath: string): string { return path1; } +/** + * Returns `true` when `candidateFsPath` is the same path as, or nested inside, `scopeFsPath` + * (inclusive of the scope itself). Both operands are resolved to absolute paths and compared with + * `path.relative`, so sibling directories sharing a name prefix (e.g. `.../app` vs `.../app-2`) are + * correctly treated as outside the scope. + */ +export function isPathInside(scopeFsPath: string, candidateFsPath: string): boolean { + const relative = path.relative(path.resolve(scopeFsPath), path.resolve(candidateFsPath)); + return ( + relative === '' || + (relative !== '..' && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative)) + ); +} + export function getResourceUri(resourcePath: string, root?: string): Uri | undefined { try { if (!resourcePath) { diff --git a/src/managers/builtin/venvManager.ts b/src/managers/builtin/venvManager.ts index 6dcda4df8..84b95776f 100644 --- a/src/managers/builtin/venvManager.ts +++ b/src/managers/builtin/venvManager.ts @@ -25,7 +25,7 @@ import { PYTHON_EXTENSION_ID } from '../../common/constants'; import { VenvManagerStrings } from '../../common/localize'; import { traceError, traceWarn } from '../../common/logging'; import { createDeferred, Deferred } from '../../common/utils/deferred'; -import { normalizePath } from '../../common/utils/pathUtils'; +import { normalizePath, isPathInside } from '../../common/utils/pathUtils'; import { showErrorMessage, showInformationMessage, withProgress } from '../../common/window.apis'; import { findParentIfFile } from '../../features/envCommands'; import { getProjectFsPathForScope, tryFastPathGet } from '../common/fastPath'; @@ -51,6 +51,7 @@ import { export class VenvManager implements EnvironmentManager { private collection: PythonEnvironment[] = []; + private refreshChain: Promise = Promise.resolve(); private readonly fsPathToEnv: Map = new Map(); private globalEnv: PythonEnvironment | undefined; private skipWatcherRefresh = false; @@ -324,28 +325,101 @@ export class VenvManager implements EnvironmentManager { title, }, async () => { - const discard = this.collection.map((env) => ({ - kind: EnvironmentChangeKind.remove, - environment: env, - })); - - this.collection = - (await findVirtualEnvironments( - hardRefresh, - this.nativeFinder, - this.api, - this.log, - this, - scope ? [scope] : undefined, - )) ?? []; - await this.loadEnvMap(); + const run = this.refreshChain.then( + async (): Promise => { + const discovered = + (await findVirtualEnvironments( + hardRefresh, + this.nativeFinder, + this.api, + this.log, + this, + scope ? [scope] : undefined, + )) ?? []; + if (scope) { + return this.mergeScopedEnvironments(scope, discovered); + } + const discard = this.collection.map((env) => ({ + kind: EnvironmentChangeKind.remove, + environment: env, + })); + this.collection = discovered; + await this.loadEnvMap(); + const added = this.collection.map((env) => ({ + environment: env, + kind: EnvironmentChangeKind.add, + })); + return [...discard, ...added]; + }, + ); + this.refreshChain = run.then( + () => undefined, + () => undefined, + ); + const changes = await run; - const added = this.collection.map((env) => ({ environment: env, kind: EnvironmentChangeKind.add })); - this._onDidChangeEnvironments.fire([...discard, ...added]); + if (changes !== undefined) { + this._onDidChangeEnvironments.fire(changes); + } }, ); } + // A scoped discovery is authoritative only within `scope`: environments in other workspace + // folders (and globals outside it) are retained untouched, so they neither disappear nor emit + // events; only in-scope environments are replaced by the freshly discovered ones. + private async mergeScopedEnvironments( + scope: Uri, + discovered: readonly PythonEnvironment[], + ): Promise { + let scopeDir: string | undefined; + try { + scopeDir = await findParentIfFile(scope.fsPath); + } catch (err) { + if (err instanceof Error && (err as NodeJS.ErrnoException).code === 'ENOENT') { + const project = this.api.getPythonProject(scope); + if (project && project.uri.fsPath !== scope.fsPath) { + scopeDir = await findParentIfFile(project.uri.fsPath).catch(() => undefined); + } + } + } + if (scopeDir === undefined) { + return undefined; + } + const scopeRoot = scopeDir; + const inScope = (env: PythonEnvironment): boolean => isPathInside(scopeRoot, env.environmentPath.fsPath); + + const retained: PythonEnvironment[] = []; + const removed: PythonEnvironment[] = []; + for (const env of this.collection) { + (inScope(env) ? removed : retained).push(env); + } + const seenPaths = new Set(retained.map((env) => normalizePath(env.environmentPath.fsPath))); + const added: PythonEnvironment[] = []; + for (const env of discovered) { + if (!inScope(env)) { + continue; + } + const key = normalizePath(env.environmentPath.fsPath); + if (seenPaths.has(key)) { + continue; + } + seenPaths.add(key); + added.push(env); + } + + this.collection = [...retained, ...added]; + const knownIds = new Set(this.collection.map((env) => env.envId.id)); + await this.loadEnvMap(); + const appended = this.collection.filter((env) => !knownIds.has(env.envId.id)); + const changes: DidChangeEnvironmentsEventArgs = [ + ...removed.map((env) => ({ kind: EnvironmentChangeKind.remove, environment: env })), + ...added.map((env) => ({ environment: env, kind: EnvironmentChangeKind.add })), + ...appended.map((env) => ({ environment: env, kind: EnvironmentChangeKind.add })), + ]; + return changes.length > 0 ? changes : undefined; + } + async getEnvironments(scope: GetEnvironmentsScope): Promise { await this.initialize(); diff --git a/src/test/common/pathUtils.unit.test.ts b/src/test/common/pathUtils.unit.test.ts index 1733e789f..bbb720ed3 100644 --- a/src/test/common/pathUtils.unit.test.ts +++ b/src/test/common/pathUtils.unit.test.ts @@ -1,7 +1,8 @@ import assert from 'node:assert'; +import * as path from 'node:path'; import * as sinon from 'sinon'; import { Uri } from 'vscode'; -import { getResourceUri, normalizePath } from '../../common/utils/pathUtils'; +import { getResourceUri, isPathInside, normalizePath } from '../../common/utils/pathUtils'; import * as utils from '../../common/utils/platformUtils'; suite('Path Utilities', () => { @@ -128,4 +129,52 @@ suite('Path Utilities', () => { assert.strictEqual(result, 'C:/Path/To/File.txt'); }); }); + + suite('isPathInside', () => { + const root = path.join(path.parse(process.cwd()).root, 'workspaces', 'app'); + + test('returns true when the candidate equals the scope (inclusive of scope.fsPath)', () => { + assert.strictEqual(isPathInside(root, root), true); + }); + + test('returns true for a direct child path', () => { + assert.strictEqual(isPathInside(root, path.join(root, '.venv')), true); + }); + + test('returns true for a deeply nested child path', () => { + assert.strictEqual(isPathInside(root, path.join(root, '.venv', 'bin', 'python')), true); + }); + + test('returns false for the parent directory', () => { + assert.strictEqual(isPathInside(root, path.dirname(root)), false); + }); + + test('returns false for a sibling directory that shares a name prefix (app vs app-2)', () => { + const sibling = path.join(path.dirname(root), 'app-2', '.venv', 'bin', 'python'); + assert.strictEqual(isPathInside(root, sibling), false); + }); + + test('returns false for an unrelated directory', () => { + const unrelated = path.join(path.dirname(root), 'other', '.venv'); + assert.strictEqual(isPathInside(root, unrelated), false); + }); + + test('resolves relative segments in the candidate before comparing', () => { + assert.strictEqual(isPathInside(root, path.join(root, 'pkg', '..', '.venv')), true); + }); + + test('returns false for a path on a different Windows drive', function () { + if (process.platform !== 'win32') { + this.skip(); + } + assert.strictEqual(isPathInside('C:\\workspaces\\app', 'D:\\workspaces\\app\\.venv'), false); + }); + + test('is case-insensitive on Windows (drive letter and folder casing)', function () { + if (process.platform !== 'win32') { + this.skip(); + } + assert.strictEqual(isPathInside('C:\\Workspaces\\App', 'c:\\workspaces\\app\\.venv\\Scripts\\python.exe'), true); + }); + }); }); diff --git a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts new file mode 100644 index 000000000..cbbb4e28c --- /dev/null +++ b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts @@ -0,0 +1,548 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import assert from 'assert'; +import * as os from 'os'; +import * as path from 'path'; +import * as sinon from 'sinon'; +import { Uri } from 'vscode'; +import { + DidChangeEnvironmentsEventArgs, + EnvironmentChangeKind, + EnvironmentManager, + PythonEnvironment, + PythonEnvironmentApi, +} from '../../../api'; +import { VENV_MANAGER_ID } from '../../../common/constants'; +import { createDeferred } from '../../../common/utils/deferred'; +import * as windowApis from '../../../common/window.apis'; +import * as envCommands from '../../../features/envCommands'; +import { VenvManager } from '../../../managers/builtin/venvManager'; +import * as venvUtils from '../../../managers/builtin/venvUtils'; +import { NativePythonFinder } from '../../../managers/common/nativePythonFinder'; +import { createMockPythonEnvironment } from '../../mocks/pythonEnvironment'; + +const ROOT = Uri.file(path.join(os.tmpdir(), 'vscode-python-envs-tests', 'venv-scoped-refresh')).fsPath; +const GLOBAL_ROOT = Uri.file(path.join(os.tmpdir(), 'vscode-python-envs-tests', 'venv-scoped-global')).fsPath; + +suite('VenvManager - scoped refresh preservation', () => { + let findVirtualEnvironmentsStub: sinon.SinonStub; + let findParentIfFileStub: sinon.SinonStub; + + const folderA = path.join(ROOT, 'app'); + const folderB = path.join(ROOT, 'app-2'); + const venvARoot = path.join(folderA, '.venv'); + const venvBRoot = path.join(folderB, '.venv'); + const globalVenvRoot = path.join(GLOBAL_ROOT, 'shared-env'); + + setup(() => { + findVirtualEnvironmentsStub = sinon.stub(venvUtils, 'findVirtualEnvironments'); + sinon.stub(venvUtils, 'getVenvForGlobal').resolves(undefined); + sinon.stub(venvUtils, 'getVenvForWorkspace').resolves(undefined); + sinon.stub(venvUtils, 'resolveVenvPythonEnvironmentPath').resolves(undefined); + findParentIfFileStub = sinon.stub(envCommands, 'findParentIfFile').callsFake(async (p: string) => p); + sinon + .stub(windowApis, 'withProgress') + .callsFake((_options: any, task: any) => + task( + { report: () => {} }, + { isCancellationRequested: false, onCancellationRequested: () => ({ dispose() {} }) }, + ), + ); + }); + + teardown(() => { + sinon.restore(); + }); + + test('retains siblings and globals while rediscovering the in-scope env', async () => { + const manager = createManager(); + const envAOld = makeEnv('A-old', venvARoot, '3.11.0'); + const envB = makeEnv('B', venvBRoot); + const envGlobal = makeEnv('G', globalVenvRoot); + seed(manager, [envAOld, envB, envGlobal]); + + const envANew = makeEnv('A-new', venvARoot, '3.12.5'); + findVirtualEnvironmentsStub.resolves([envANew]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + const collection: PythonEnvironment[] = (manager as any).collection; + assert.deepStrictEqual(ids(collection), ['A-new', 'B', 'G']); + assert.strictEqual(collection.length, 3); + assert.ok(collection.includes(envB) && collection.includes(envGlobal) && collection.includes(envANew)); + assert.ok(!collection.includes(envAOld)); + + const changes = flatChanges(events); + assert.deepStrictEqual( + changes.map((c) => ({ id: c.environment.envId.id, kind: c.kind })), + [ + { id: 'A-old', kind: EnvironmentChangeKind.remove }, + { id: 'A-new', kind: EnvironmentChangeKind.add }, + ], + ); + }); + + test('removes only stale environments inside the target scope', async () => { + const manager = createManager(); + seed(manager, [makeEnv('A', venvARoot), makeEnv('B', venvBRoot), makeEnv('G', globalVenvRoot)]); + findVirtualEnvironmentsStub.resolves([]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + assert.deepStrictEqual(ids((manager as any).collection), ['B', 'G']); + const changes = flatChanges(events); + assert.strictEqual(changes.length, 1); + assert.strictEqual(changes[0].kind, EnvironmentChangeKind.remove); + assert.strictEqual(changes[0].environment.envId.id, 'A'); + }); + + test('adds newly discovered environments in the target scope', async () => { + const manager = createManager(); + seed(manager, [makeEnv('B', venvBRoot)]); + + const envANew = makeEnv('A-new', venvARoot); + findVirtualEnvironmentsStub.resolves([envANew]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + assert.deepStrictEqual(ids((manager as any).collection), ['A-new', 'B']); + const changes = flatChanges(events); + assert.strictEqual(changes.length, 1); + assert.strictEqual(changes[0].kind, EnvironmentChangeKind.add); + assert.strictEqual(changes[0].environment.envId.id, 'A-new'); + }); + + test('ignores out-of-scope discovery results (configured global venvFolders) already retained', async () => { + const manager = createManager(); + const envGlobal = makeEnv('G', globalVenvRoot); + seed(manager, [envGlobal]); + + const envANew = makeEnv('A-new', venvARoot); + const envGlobalFresh = makeEnv('G-fresh', globalVenvRoot); + findVirtualEnvironmentsStub.resolves([envANew, envGlobalFresh]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + const collection: PythonEnvironment[] = (manager as any).collection; + assert.deepStrictEqual(ids(collection), ['A-new', 'G']); + assert.ok(collection.includes(envGlobal)); + assert.ok(!collection.some((e) => e.envId.id === 'G-fresh')); + + const changes = flatChanges(events); + assert.deepStrictEqual( + changes.map((c) => ({ id: c.environment.envId.id, kind: c.kind })), + [{ id: 'A-new', kind: EnvironmentChangeKind.add }], + ); + }); + + test('does not admit newly discovered out-of-scope global environments', async () => { + const manager = createManager(); + seed(manager, [makeEnv('B', venvBRoot)]); + + findVirtualEnvironmentsStub.resolves([makeEnv('A-new', venvARoot), makeEnv('G-new', globalVenvRoot)]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + const collection: PythonEnvironment[] = (manager as any).collection; + assert.deepStrictEqual(ids(collection), ['A-new', 'B']); + assert.ok(!collection.some((e) => e.envId.id === 'G-new')); + + const changes = flatChanges(events); + assert.deepStrictEqual( + changes.map((c) => ({ id: c.environment.envId.id, kind: c.kind })), + [{ id: 'A-new', kind: EnvironmentChangeKind.add }], + ); + }); + + test('deduplicates discovered environments that share a normalized path', async () => { + const manager = createManager(); + seed(manager, [makeEnv('B', venvBRoot)]); + + findVirtualEnvironmentsStub.resolves([makeEnv('A-dup-1', venvARoot), makeEnv('A-dup-2', venvARoot)]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + assert.deepStrictEqual(ids((manager as any).collection), ['A-dup-1', 'B']); + const changes = flatChanges(events); + assert.deepStrictEqual( + changes.map((c) => ({ id: c.environment.envId.id, kind: c.kind })), + [{ id: 'A-dup-1', kind: EnvironmentChangeKind.add }], + ); + }); + + test('treats sibling directories sharing a name prefix as outside the scope (app vs app-2)', async () => { + const manager = createManager(); + seed(manager, [makeEnv('B', venvBRoot)]); + findVirtualEnvironmentsStub.resolves([]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + assert.deepStrictEqual(ids((manager as any).collection), ['B']); + assert.deepStrictEqual(events, []); + }); + + test('scopes a nested directory refresh to that directory, not the whole owning project', async () => { + const manager = createManager(); + const pkgVenv = path.join(folderA, 'pkg', '.venv'); + const otherVenv = path.join(folderA, 'other', '.venv'); + seed(manager, [makeEnv('PKG-old', pkgVenv), makeEnv('OTHER', otherVenv)]); + + ((manager as any).api.getPythonProject as sinon.SinonStub).returns({ uri: Uri.file(folderA) }); + findVirtualEnvironmentsStub.resolves([makeEnv('PKG-new', pkgVenv)]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(path.join(folderA, 'pkg'))); + + assert.deepStrictEqual(ids((manager as any).collection), ['OTHER', 'PKG-new']); + const changes = flatChanges(events) + .map((c) => ({ id: c.environment.envId.id, kind: c.kind })) + .sort((a, b) => a.id.localeCompare(b.id)); + assert.deepStrictEqual(changes, [ + { id: 'PKG-new', kind: EnvironmentChangeKind.add }, + { id: 'PKG-old', kind: EnvironmentChangeKind.remove }, + ]); + }); + + test('resolves a file scope to its containing directory when no project owns it', async () => { + const manager = createManager(); + seed(manager, [makeEnv('B', venvBRoot)]); + + const fileScope = Uri.file(path.join(folderA, 'main.py')); + findParentIfFileStub.callsFake(async () => folderA); + findVirtualEnvironmentsStub.resolves([makeEnv('A-new', venvARoot)]); + + await manager.refresh(fileScope); + + assert.deepStrictEqual(ids((manager as any).collection), ['A-new', 'B']); + }); + + test('resolves a deleted file scope through its owning project when inspection fails', async () => { + const manager = createManager(); + seed(manager, [makeEnv('A-old', venvARoot), makeEnv('B', venvBRoot)]); + + const deletedScope = Uri.file(path.join(folderA, 'main.py')); + ((manager as any).api.getPythonProject as sinon.SinonStub).returns({ uri: Uri.file(folderA) }); + findParentIfFileStub.callsFake(async (p: string) => { + if (p === deletedScope.fsPath) { + throw Object.assign(new Error('ENOENT: no such file or directory'), { code: 'ENOENT' }); + } + return p; + }); + findVirtualEnvironmentsStub.resolves([makeEnv('A-new', venvARoot)]); + + await manager.refresh(deletedScope); + + assert.deepStrictEqual(ids((manager as any).collection), ['A-new', 'B']); + }); + + test('normalizes a file-based owning project uri to its directory when the scope is uninspectable', async () => { + const manager = createManager(); + seed(manager, [makeEnv('A-old', venvARoot), makeEnv('B', venvBRoot)]); + + const deletedScope = Uri.file(path.join(folderA, 'deleted.py')); + const projectFile = path.join(folderA, 'app.py'); + ((manager as any).api.getPythonProject as sinon.SinonStub).returns({ uri: Uri.file(projectFile) }); + findParentIfFileStub.callsFake(async (p: string) => { + if (p === deletedScope.fsPath) { + throw Object.assign(new Error('ENOENT: no such file or directory'), { code: 'ENOENT' }); + } + return path.dirname(p); + }); + findVirtualEnvironmentsStub.resolves([makeEnv('A-new', venvARoot)]); + + await manager.refresh(deletedScope); + + assert.deepStrictEqual(ids((manager as any).collection), ['A-new', 'B']); + }); + + test('skips scoped mutation without widening when an uninspectable directory scope equals its owning project uri', async () => { + const manager = createManager(); + seed(manager, [makeEnv('A', venvARoot), makeEnv('B', venvBRoot)]); + + ((manager as any).api.getPythonProject as sinon.SinonStub).returns({ uri: Uri.file(folderA) }); + findParentIfFileStub.rejects(Object.assign(new Error('ENOENT: no such file or directory'), { code: 'ENOENT' })); + findVirtualEnvironmentsStub.resolves([makeEnv('A-new', venvARoot)]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + assert.deepStrictEqual(ids((manager as any).collection), ['A', 'B']); + assert.deepStrictEqual(events, []); + }); + + test('skips scoped mutation for an uninspectable file scope that no project owns', async () => { + const manager = createManager(); + seed(manager, [makeEnv('A-old', venvARoot), makeEnv('B', venvBRoot)]); + + findParentIfFileStub.rejects(Object.assign(new Error('ENOENT: no such file or directory'), { code: 'ENOENT' })); + findVirtualEnvironmentsStub.resolves([makeEnv('A-new', venvARoot)]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(path.join(folderA, 'deleted.py'))); + + assert.deepStrictEqual(ids((manager as any).collection), ['A-old', 'B']); + assert.deepStrictEqual(events, []); + }); + + test('skips reconciliation without widening when a nested scope inspection fails with EACCES', async () => { + const manager = createManager(); + const pkgVenv = path.join(folderA, 'pkg', '.venv'); + const otherVenv = path.join(folderA, 'other', '.venv'); + seed(manager, [makeEnv('PKG', pkgVenv), makeEnv('OTHER', otherVenv)]); + + const nestedScope = Uri.file(path.join(folderA, 'pkg')); + ((manager as any).api.getPythonProject as sinon.SinonStub).returns({ uri: Uri.file(folderA) }); + findParentIfFileStub.callsFake(async (p: string) => { + if (p === nestedScope.fsPath) { + throw Object.assign(new Error('EACCES: permission denied'), { code: 'EACCES' }); + } + return p; + }); + findVirtualEnvironmentsStub.resolves([makeEnv('PKG-new', pkgVenv)]); + + const events = captureEvents(manager); + await manager.refresh(nestedScope); + + assert.deepStrictEqual(ids((manager as any).collection), ['OTHER', 'PKG']); + assert.deepStrictEqual(events, []); + }); + + test('announces an out-of-scope environment appended while loading the project map', async () => { + const manager = createManager(); + seed(manager, []); + + findVirtualEnvironmentsStub.resolves([makeEnv('A-new', venvARoot)]); + ((manager as any).api.getPythonProjects as sinon.SinonStub).returns([{ uri: Uri.file(folderB) }]); + (venvUtils.getVenvForWorkspace as sinon.SinonStub).resolves(venvPython(venvBRoot)); + (venvUtils.resolveVenvPythonEnvironmentPath as sinon.SinonStub).resolves(makeEnv('B-PERSISTED', venvBRoot)); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + assert.deepStrictEqual(ids((manager as any).collection), ['A-new', 'B-PERSISTED']); + const changes = flatChanges(events); + assert.deepStrictEqual( + changes.map((c) => ({ id: c.environment.envId.id, kind: c.kind })), + [ + { id: 'A-new', kind: EnvironmentChangeKind.add }, + { id: 'B-PERSISTED', kind: EnvironmentChangeKind.add }, + ], + ); + }); + + test('a scoped refresh announces an in-scope environment appended while loading the project map', async () => { + const manager = createManager(); + seed(manager, [makeEnv('B', venvBRoot)]); + + findVirtualEnvironmentsStub.resolves([]); + ((manager as any).api.getPythonProjects as sinon.SinonStub).returns([{ uri: Uri.file(folderA) }]); + (venvUtils.getVenvForWorkspace as sinon.SinonStub).resolves(venvPython(venvARoot)); + (venvUtils.resolveVenvPythonEnvironmentPath as sinon.SinonStub).resolves(makeEnv('PERSISTED', venvARoot)); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + assert.deepStrictEqual(ids((manager as any).collection), ['B', 'PERSISTED']); + const changes = flatChanges(events); + assert.deepStrictEqual( + changes.map((c) => ({ id: c.environment.envId.id, kind: c.kind })), + [{ id: 'PERSISTED', kind: EnvironmentChangeKind.add }], + ); + }); + + test('a full refresh in progress does not announce results discovered by a concurrent scoped refresh', async () => { + const manager = createManager(); + seed(manager, []); + + const xRoot = path.join(ROOT, 'x', '.venv'); + const yRoot = path.join(ROOT, 'y', '.venv'); + findVirtualEnvironmentsStub.callsFake(async (...args: any[]) => { + const uris = args[5] as Uri[] | undefined; + return uris ? [makeEnv('A-new', venvARoot)] : [makeEnv('X', xRoot), makeEnv('Y', yRoot)]; + }); + + const fullInLoadEnvMap = createDeferred(); + const releaseFull = createDeferred(); + let call = 0; + ((manager as any).baseManager.getEnvironments as sinon.SinonStub).callsFake(async () => { + call += 1; + if (call === 1) { + fullInLoadEnvMap.resolve(); + await releaseFull.promise; + } + return []; + }); + + const events = captureEvents(manager); + const pFull = manager.refresh(undefined); + await fullInLoadEnvMap.promise; + const pScoped = manager.refresh(Uri.file(folderA)); + await new Promise((resolve) => setImmediate(resolve)); + releaseFull.resolve(); + await Promise.all([pFull, pScoped]); + + assert.deepStrictEqual(ids((manager as any).collection), ['A-new', 'X', 'Y']); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [ + [ + { id: 'X', kind: EnvironmentChangeKind.add }, + { id: 'Y', kind: EnvironmentChangeKind.add }, + ], + [{ id: 'A-new', kind: EnvironmentChangeKind.add }], + ], + ); + }); + + test('applies overlapping full refreshes in invocation order even when the first discovery is delayed', async () => { + const manager = createManager(); + seed(manager, []); + + const gateFirst = createDeferred(); + let call = 0; + findVirtualEnvironmentsStub.callsFake(async () => { + call += 1; + if (call === 1) { + await gateFirst.promise; + return [makeEnv('FIRST', venvARoot)]; + } + return [makeEnv('SECOND', venvBRoot)]; + }); + + const events = captureEvents(manager); + const pFirst = manager.refresh(undefined); + const pSecond = manager.refresh(undefined); + gateFirst.resolve(); + await Promise.all([pFirst, pSecond]); + + assert.deepStrictEqual(ids((manager as any).collection), ['SECOND']); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [ + [{ id: 'FIRST', kind: EnvironmentChangeKind.add }], + [ + { id: 'FIRST', kind: EnvironmentChangeKind.remove }, + { id: 'SECOND', kind: EnvironmentChangeKind.add }, + ], + ], + ); + }); + + test('keeps the refresh chain usable after a discovery failure', async () => { + const manager = createManager(); + seed(manager, []); + + let call = 0; + findVirtualEnvironmentsStub.callsFake(async () => { + call += 1; + if (call === 1) { + throw new Error('discovery failed'); + } + return [makeEnv('RECOVERED', venvARoot)]; + }); + + const events = captureEvents(manager); + await assert.rejects(manager.refresh(undefined), /discovery failed/); + await manager.refresh(undefined); + + assert.deepStrictEqual(ids((manager as any).collection), ['RECOVERED']); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [[{ id: 'RECOVERED', kind: EnvironmentChangeKind.add }]], + ); + }); + + test('full (unscoped) refresh still replaces the entire collection', async () => { + const manager = createManager(); + seed(manager, [makeEnv('A', venvARoot), makeEnv('B', venvBRoot)]); + + const envX = makeEnv('X', path.join(ROOT, 'x', '.venv')); + const envY = makeEnv('Y', path.join(ROOT, 'y', '.venv')); + findVirtualEnvironmentsStub.resolves([envX, envY]); + + const events = captureEvents(manager); + await manager.refresh(undefined); + + assert.deepStrictEqual(ids((manager as any).collection), ['X', 'Y'].sort()); + + const changes = flatChanges(events); + const removed = changes + .filter((c) => c.kind === EnvironmentChangeKind.remove) + .map((c) => c.environment.envId.id); + const added = changes.filter((c) => c.kind === EnvironmentChangeKind.add).map((c) => c.environment.envId.id); + assert.deepStrictEqual(removed.sort(), ['A', 'B']); + assert.deepStrictEqual(added.sort(), ['X', 'Y']); + }); + + test('passes the scope through to discovery as a single-element uri array', async () => { + const manager = createManager(); + seed(manager, []); + findVirtualEnvironmentsStub.resolves([]); + + const scope = Uri.file(folderA); + await manager.refresh(scope); + + const uris = findVirtualEnvironmentsStub.firstCall.args[5] as Uri[] | undefined; + assert.ok(Array.isArray(uris) && uris.length === 1); + assert.strictEqual(uris[0].fsPath, scope.fsPath); + }); + + function createManager(): VenvManager { + const api = { + getEnvironments: sinon.stub().resolves([]), + getPythonProject: sinon.stub().returns(undefined), + getPythonProjects: sinon.stub().returns([]), + refreshEnvironments: sinon.stub().resolves(undefined), + } as any as PythonEnvironmentApi; + const baseManager = { + getEnvironments: sinon.stub().resolves([]), + } as any as EnvironmentManager; + const manager = new VenvManager({} as NativePythonFinder, api, baseManager, { + info: sinon.stub(), + error: sinon.stub(), + warn: sinon.stub(), + } as any); + (manager as any)._initialized = { promise: Promise.resolve() }; + (manager as any).collection = []; + return manager; + } + + function seed(manager: VenvManager, envs: PythonEnvironment[]): void { + (manager as any).collection = envs; + } + + function captureEvents(manager: VenvManager): DidChangeEnvironmentsEventArgs[] { + const events: DidChangeEnvironmentsEventArgs[] = []; + manager.onDidChangeEnvironments((e) => events.push(e)); + return events; + } + + function flatChanges(events: DidChangeEnvironmentsEventArgs[]): DidChangeEnvironmentsEventArgs { + return events.flat(); + } +}); + +function venvPython(venvRoot: string): string { + return path.join(venvRoot, process.platform === 'win32' ? 'Scripts' : 'bin', 'python'); +} + +function makeEnv(id: string, venvRoot: string, version?: string): PythonEnvironment { + return createMockPythonEnvironment({ + name: path.basename(venvRoot), + envPath: venvPython(venvRoot), + sysPrefix: venvRoot, + managerId: VENV_MANAGER_ID, + id, + version, + }); +} + +function ids(collection: PythonEnvironment[]): string[] { + return collection.map((e) => e.envId.id).sort(); +} From d9cdf7f145e2db07e6cdbb5a28cc2951b3d8e6e7 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Sun, 23 Aug 2026 12:48:50 -0700 Subject: [PATCH 2/9] fix: preserve scope authority and operation-owned events in venv scoped refresh Skip scoped reconciliation when the requested scope is uninspectable instead of widening authority to the owning project root, and derive scoped add events from loadEnvMap's own appends with post-await revalidation so concurrent create/remove are not misattributed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/managers/builtin/venvManager.ts | 64 ++++++++----- .../venvManager.scopedRefresh.unit.test.ts | 95 ++++++++++++++++--- 2 files changed, 121 insertions(+), 38 deletions(-) diff --git a/src/managers/builtin/venvManager.ts b/src/managers/builtin/venvManager.ts index 84b95776f..043e2e286 100644 --- a/src/managers/builtin/venvManager.ts +++ b/src/managers/builtin/venvManager.ts @@ -372,21 +372,12 @@ export class VenvManager implements EnvironmentManager { scope: Uri, discovered: readonly PythonEnvironment[], ): Promise { - let scopeDir: string | undefined; + let scopeRoot: string; try { - scopeDir = await findParentIfFile(scope.fsPath); - } catch (err) { - if (err instanceof Error && (err as NodeJS.ErrnoException).code === 'ENOENT') { - const project = this.api.getPythonProject(scope); - if (project && project.uri.fsPath !== scope.fsPath) { - scopeDir = await findParentIfFile(project.uri.fsPath).catch(() => undefined); - } - } - } - if (scopeDir === undefined) { + scopeRoot = await findParentIfFile(scope.fsPath); + } catch { return undefined; } - const scopeRoot = scopeDir; const inScope = (env: PythonEnvironment): boolean => isPathInside(scopeRoot, env.environmentPath.fsPath); const retained: PythonEnvironment[] = []; @@ -409,13 +400,23 @@ export class VenvManager implements EnvironmentManager { } this.collection = [...retained, ...added]; - const knownIds = new Set(this.collection.map((env) => env.envId.id)); - await this.loadEnvMap(); - const appended = this.collection.filter((env) => !knownIds.has(env.envId.id)); + const appended = await this.loadEnvMap(); + const currentIds = new Set(this.collection.map((env) => env.envId.id)); + + const seenAddIds = new Set(); + const addChanges: DidChangeEnvironmentsEventArgs = []; + for (const env of [...added, ...appended]) { + if (!currentIds.has(env.envId.id) || seenAddIds.has(env.envId.id)) { + continue; + } + seenAddIds.add(env.envId.id); + addChanges.push({ environment: env, kind: EnvironmentChangeKind.add }); + } const changes: DidChangeEnvironmentsEventArgs = [ - ...removed.map((env) => ({ kind: EnvironmentChangeKind.remove, environment: env })), - ...added.map((env) => ({ environment: env, kind: EnvironmentChangeKind.add })), - ...appended.map((env) => ({ environment: env, kind: EnvironmentChangeKind.add })), + ...removed + .filter((env) => !currentIds.has(env.envId.id)) + .map((env) => ({ kind: EnvironmentChangeKind.remove, environment: env })), + ...addChanges, ]; return changes.length > 0 ? changes : undefined; } @@ -581,9 +582,9 @@ export class VenvManager implements EnvironmentManager { await clearVenvCache(); } - private addEnvironment(environment: PythonEnvironment, raiseEvent?: boolean): void { + private addEnvironment(environment: PythonEnvironment, raiseEvent?: boolean): PythonEnvironment | undefined { if (this.collection.find((e) => e.envId.id === environment.envId.id)) { - return; + return undefined; } const oldEnv = this.findEnvironmentByPath(environment.environmentPath.fsPath); @@ -602,6 +603,7 @@ export class VenvManager implements EnvironmentManager { this._onDidChangeEnvironments.fire([{ environment, kind: EnvironmentChangeKind.add }]); } } + return environment; } private async resetGlobalEnv() { @@ -613,12 +615,13 @@ export class VenvManager implements EnvironmentManager { /** * Loads and sets the global Python environment from the provided list, resolving if necessary. O(g) where g = globals.length */ - private async loadGlobalEnv(globals: PythonEnvironment[]) { + private async loadGlobalEnv(globals: PythonEnvironment[]): Promise { this.globalEnv = undefined; // Try to find a global environment const fsPath = await getVenvForGlobal(); + let added: PythonEnvironment | undefined; if (fsPath) { this.globalEnv = this.findEnvironmentByPath(fsPath) ?? this.findEnvironmentByPath(fsPath, globals); @@ -634,7 +637,7 @@ export class VenvManager implements EnvironmentManager { // If the environment is resolved, add it to the collection if (this.globalEnv) { - this.addEnvironment(this.globalEnv, false); + added = this.addEnvironment(this.globalEnv, false); } } } @@ -643,14 +646,19 @@ export class VenvManager implements EnvironmentManager { if (!this.globalEnv) { this.globalEnv = getLatest(globals); } + return added; } /** * Loads and maps Python environments to their corresponding project paths in the workspace. about O(p × e) where p = projects.len and e = environments.len */ - private async loadEnvMap() { + private async loadEnvMap(): Promise { + const appended: PythonEnvironment[] = []; const globals = await this.baseManager.getEnvironments('global'); - await this.loadGlobalEnv(globals); + const globalAdded = await this.loadGlobalEnv(globals); + if (globalAdded) { + appended.push(globalAdded); + } this.fsPathToEnv.clear(); @@ -677,11 +685,14 @@ export class VenvManager implements EnvironmentManager { ); if (resolved) { // If resolved; add it to the venvManager collection - this.addEnvironment(resolved, false); + const addedEnv = this.addEnvironment(resolved, false); + if (addedEnv) { + appended.push(addedEnv); + } foundEnv = resolved; } else { this.log.error(`Failed to resolve python environment: ${env}`); - return; + return appended; } } // Given found env, add it to the map and fire the event if needed. @@ -704,6 +715,7 @@ export class VenvManager implements EnvironmentManager { } events.forEach((e) => e()); + return appended; } /** diff --git a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts index cbbb4e28c..80424b69d 100644 --- a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts +++ b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts @@ -222,7 +222,7 @@ suite('VenvManager - scoped refresh preservation', () => { assert.deepStrictEqual(ids((manager as any).collection), ['A-new', 'B']); }); - test('resolves a deleted file scope through its owning project when inspection fails', async () => { + test('skips scoped mutation for a deleted file scope owned by a project', async () => { const manager = createManager(); seed(manager, [makeEnv('A-old', venvARoot), makeEnv('B', venvBRoot)]); @@ -236,29 +236,34 @@ suite('VenvManager - scoped refresh preservation', () => { }); findVirtualEnvironmentsStub.resolves([makeEnv('A-new', venvARoot)]); + const events = captureEvents(manager); await manager.refresh(deletedScope); - assert.deepStrictEqual(ids((manager as any).collection), ['A-new', 'B']); + assert.deepStrictEqual(ids((manager as any).collection), ['A-old', 'B']); + assert.deepStrictEqual(events, []); }); - test('normalizes a file-based owning project uri to its directory when the scope is uninspectable', async () => { + test('skips scoped mutation for a missing nested directory scope owned by a project', async () => { const manager = createManager(); - seed(manager, [makeEnv('A-old', venvARoot), makeEnv('B', venvBRoot)]); + const pkgVenv = path.join(folderA, 'pkg', '.venv'); + const otherVenv = path.join(folderA, 'other', '.venv'); + seed(manager, [makeEnv('PKG', pkgVenv), makeEnv('OTHER', otherVenv)]); - const deletedScope = Uri.file(path.join(folderA, 'deleted.py')); - const projectFile = path.join(folderA, 'app.py'); - ((manager as any).api.getPythonProject as sinon.SinonStub).returns({ uri: Uri.file(projectFile) }); + const missingNested = Uri.file(path.join(folderA, 'pkg')); + ((manager as any).api.getPythonProject as sinon.SinonStub).returns({ uri: Uri.file(folderA) }); findParentIfFileStub.callsFake(async (p: string) => { - if (p === deletedScope.fsPath) { + if (p === missingNested.fsPath) { throw Object.assign(new Error('ENOENT: no such file or directory'), { code: 'ENOENT' }); } - return path.dirname(p); + return p; }); - findVirtualEnvironmentsStub.resolves([makeEnv('A-new', venvARoot)]); + findVirtualEnvironmentsStub.resolves([makeEnv('PKG-new', pkgVenv)]); - await manager.refresh(deletedScope); + const events = captureEvents(manager); + await manager.refresh(missingNested); - assert.deepStrictEqual(ids((manager as any).collection), ['A-new', 'B']); + assert.deepStrictEqual(ids((manager as any).collection), ['OTHER', 'PKG']); + assert.deepStrictEqual(events, []); }); test('skips scoped mutation without widening when an uninspectable directory scope equals its owning project uri', async () => { @@ -458,6 +463,72 @@ suite('VenvManager - scoped refresh preservation', () => { ); }); + test('does not attribute a concurrent create to a scoped refresh while loading the project map', async () => { + const manager = createManager(); + seed(manager, []); + + findVirtualEnvironmentsStub.resolves([makeEnv('A', venvARoot)]); + + const inLoadEnvMap = createDeferred(); + const release = createDeferred(); + let call = 0; + ((manager as any).baseManager.getEnvironments as sinon.SinonStub).callsFake(async () => { + call += 1; + if (call === 1) { + inLoadEnvMap.resolve(); + await release.promise; + } + return []; + }); + + const events = captureEvents(manager); + const pRefresh = manager.refresh(Uri.file(folderA)); + await inLoadEnvMap.promise; + (manager as any).addEnvironment(makeEnv('CREATED', path.join(ROOT, 'created', '.venv')), true); + release.resolve(); + await pRefresh; + + assert.deepStrictEqual(ids((manager as any).collection), ['A', 'CREATED']); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [[{ id: 'CREATED', kind: EnvironmentChangeKind.add }], [{ id: 'A', kind: EnvironmentChangeKind.add }]], + ); + }); + + test('does not publish a stale add when a scoped refresh discovery is removed during project map loading', async () => { + const manager = createManager(); + seed(manager, []); + + const envA = makeEnv('A', venvARoot); + findVirtualEnvironmentsStub.resolves([envA]); + sinon.stub(venvUtils, 'removeVenv').resolves(true); + + const inLoadEnvMap = createDeferred(); + const release = createDeferred(); + let call = 0; + ((manager as any).baseManager.getEnvironments as sinon.SinonStub).callsFake(async () => { + call += 1; + if (call === 1) { + inLoadEnvMap.resolve(); + await release.promise; + } + return []; + }); + + const events = captureEvents(manager); + const pRefresh = manager.refresh(Uri.file(folderA)); + await inLoadEnvMap.promise; + await manager.remove(envA); + release.resolve(); + await pRefresh; + + assert.deepStrictEqual(ids((manager as any).collection), []); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [[{ id: 'A', kind: EnvironmentChangeKind.remove }]], + ); + }); + test('full (unscoped) refresh still replaces the entire collection', async () => { const manager = createManager(); seed(manager, [makeEnv('A', venvARoot), makeEnv('B', venvBRoot)]); From 075d941e31372d0791808d5c844b4c56f52b1035 Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Sun, 23 Aug 2026 14:05:29 -0700 Subject: [PATCH 3/9] fix: keep venv scoped refresh in-scope for map loading and add ownership Prevent scoped project-map loading from appending out-of-scope persisted or global environments by threading an in-scope predicate into loadEnvMap and loadGlobalEnv, and derive scoped add events from collection object identity so a same-id remove-and-recreate during map loading cannot emit a duplicate add. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/managers/builtin/venvManager.ts | 24 ++++--- .../venvManager.scopedRefresh.unit.test.ts | 67 ++++++++++++++++--- 2 files changed, 74 insertions(+), 17 deletions(-) diff --git a/src/managers/builtin/venvManager.ts b/src/managers/builtin/venvManager.ts index 043e2e286..50a4c89ac 100644 --- a/src/managers/builtin/venvManager.ts +++ b/src/managers/builtin/venvManager.ts @@ -378,17 +378,17 @@ export class VenvManager implements EnvironmentManager { } catch { return undefined; } - const inScope = (env: PythonEnvironment): boolean => isPathInside(scopeRoot, env.environmentPath.fsPath); + const inScope = (fsPath: string): boolean => isPathInside(scopeRoot, fsPath); const retained: PythonEnvironment[] = []; const removed: PythonEnvironment[] = []; for (const env of this.collection) { - (inScope(env) ? removed : retained).push(env); + (inScope(env.environmentPath.fsPath) ? removed : retained).push(env); } const seenPaths = new Set(retained.map((env) => normalizePath(env.environmentPath.fsPath))); const added: PythonEnvironment[] = []; for (const env of discovered) { - if (!inScope(env)) { + if (!inScope(env.environmentPath.fsPath)) { continue; } const key = normalizePath(env.environmentPath.fsPath); @@ -400,13 +400,13 @@ export class VenvManager implements EnvironmentManager { } this.collection = [...retained, ...added]; - const appended = await this.loadEnvMap(); + const appended = await this.loadEnvMap(inScope); const currentIds = new Set(this.collection.map((env) => env.envId.id)); const seenAddIds = new Set(); const addChanges: DidChangeEnvironmentsEventArgs = []; for (const env of [...added, ...appended]) { - if (!currentIds.has(env.envId.id) || seenAddIds.has(env.envId.id)) { + if (!this.collection.includes(env) || seenAddIds.has(env.envId.id)) { continue; } seenAddIds.add(env.envId.id); @@ -615,7 +615,10 @@ export class VenvManager implements EnvironmentManager { /** * Loads and sets the global Python environment from the provided list, resolving if necessary. O(g) where g = globals.length */ - private async loadGlobalEnv(globals: PythonEnvironment[]): Promise { + private async loadGlobalEnv( + globals: PythonEnvironment[], + inScope?: (fsPath: string) => boolean, + ): Promise { this.globalEnv = undefined; // Try to find a global environment @@ -636,7 +639,7 @@ export class VenvManager implements EnvironmentManager { ); // If the environment is resolved, add it to the collection - if (this.globalEnv) { + if (this.globalEnv && (!inScope || inScope(this.globalEnv.environmentPath.fsPath))) { added = this.addEnvironment(this.globalEnv, false); } } @@ -652,10 +655,10 @@ export class VenvManager implements EnvironmentManager { /** * Loads and maps Python environments to their corresponding project paths in the workspace. about O(p × e) where p = projects.len and e = environments.len */ - private async loadEnvMap(): Promise { + private async loadEnvMap(inScope?: (fsPath: string) => boolean): Promise { const appended: PythonEnvironment[] = []; const globals = await this.baseManager.getEnvironments('global'); - const globalAdded = await this.loadGlobalEnv(globals); + const globalAdded = await this.loadGlobalEnv(globals, inScope); if (globalAdded) { appended.push(globalAdded); } @@ -675,6 +678,9 @@ export class VenvManager implements EnvironmentManager { let foundEnv = this.findEnvironmentByPath(env, sorted) ?? this.findEnvironmentByPath(env, globals); const previousEnv = this.fsPathToEnv.get(normalizedPath); if (!foundEnv) { + if (inScope && !inScope(env)) { + continue; + } // attempt to resolve const resolved = await resolveVenvPythonEnvironmentPath( env, diff --git a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts index 80424b69d..a53ff2d83 100644 --- a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts +++ b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts @@ -318,7 +318,7 @@ suite('VenvManager - scoped refresh preservation', () => { assert.deepStrictEqual(events, []); }); - test('announces an out-of-scope environment appended while loading the project map', async () => { + test('does not append an out-of-scope environment while loading the project map', async () => { const manager = createManager(); seed(manager, []); @@ -330,14 +330,28 @@ suite('VenvManager - scoped refresh preservation', () => { const events = captureEvents(manager); await manager.refresh(Uri.file(folderA)); - assert.deepStrictEqual(ids((manager as any).collection), ['A-new', 'B-PERSISTED']); - const changes = flatChanges(events); + assert.deepStrictEqual(ids((manager as any).collection), ['A-new']); assert.deepStrictEqual( - changes.map((c) => ({ id: c.environment.envId.id, kind: c.kind })), - [ - { id: 'A-new', kind: EnvironmentChangeKind.add }, - { id: 'B-PERSISTED', kind: EnvironmentChangeKind.add }, - ], + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [[{ id: 'A-new', kind: EnvironmentChangeKind.add }]], + ); + }); + + test('does not append a persisted global environment while loading the project map', async () => { + const manager = createManager(); + seed(manager, []); + + findVirtualEnvironmentsStub.resolves([makeEnv('A-new', venvARoot)]); + (venvUtils.getVenvForGlobal as sinon.SinonStub).resolves(venvPython(globalVenvRoot)); + (venvUtils.resolveVenvPythonEnvironmentPath as sinon.SinonStub).resolves(makeEnv('G-PERSISTED', globalVenvRoot)); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + assert.deepStrictEqual(ids((manager as any).collection), ['A-new']); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [[{ id: 'A-new', kind: EnvironmentChangeKind.add }]], ); }); @@ -529,6 +543,43 @@ suite('VenvManager - scoped refresh preservation', () => { ); }); + test('does not emit a duplicate add when a discovered env is removed and recreated with the same id during project map loading', async () => { + const manager = createManager(); + seed(manager, []); + + const envA = makeEnv('A', venvARoot); + findVirtualEnvironmentsStub.resolves([envA]); + sinon.stub(venvUtils, 'removeVenv').resolves(true); + + const inLoadEnvMap = createDeferred(); + const release = createDeferred(); + let call = 0; + ((manager as any).baseManager.getEnvironments as sinon.SinonStub).callsFake(async () => { + call += 1; + if (call === 1) { + inLoadEnvMap.resolve(); + await release.promise; + } + return []; + }); + + const recreated = makeEnv('A', venvARoot); + const events = captureEvents(manager); + const pRefresh = manager.refresh(Uri.file(folderA)); + await inLoadEnvMap.promise; + await manager.remove(envA); + (manager as any).addEnvironment(recreated, true); + release.resolve(); + await pRefresh; + + assert.strictEqual((manager as any).collection.length, 1); + assert.strictEqual((manager as any).collection[0], recreated); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [[{ id: 'A', kind: EnvironmentChangeKind.remove }], [{ id: 'A', kind: EnvironmentChangeKind.add }]], + ); + }); + test('full (unscoped) refresh still replaces the entire collection', async () => { const manager = createManager(); seed(manager, [makeEnv('A', venvARoot), makeEnv('B', venvBRoot)]); From cb2fc12eea9d0373c0c1aed436172df00006518d Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Sun, 23 Aug 2026 17:55:55 -0700 Subject: [PATCH 4/9] fix: reconcile venv scoped refresh by path identity and resolve scope before discovery Reconcile in-scope environments by normalized path plus object identity so an unchanged object emits no event, a same-path replacement (even with an equal envId.id) emits remove+add, and every stale in-scope object leaves the collection with a remove. Resolve the authoritative scope root via findParentIfFile before discovery inside the serialized refresh chain, pass the resolved directory to both discovery and reconciliation, and stay fail-closed (skip discovery and mutation) when resolution throws. Full refresh unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/managers/builtin/venvManager.ts | 79 +++++++++------- .../venvManager.scopedRefresh.unit.test.ts | 92 ++++++++++++++++++- 2 files changed, 138 insertions(+), 33 deletions(-) diff --git a/src/managers/builtin/venvManager.ts b/src/managers/builtin/venvManager.ts index 50a4c89ac..ddb159b73 100644 --- a/src/managers/builtin/venvManager.ts +++ b/src/managers/builtin/venvManager.ts @@ -327,6 +327,14 @@ export class VenvManager implements EnvironmentManager { async () => { const run = this.refreshChain.then( async (): Promise => { + let scopeRoot: string | undefined; + if (scope) { + try { + scopeRoot = await findParentIfFile(scope.fsPath); + } catch { + return undefined; + } + } const discovered = (await findVirtualEnvironments( hardRefresh, @@ -334,10 +342,10 @@ export class VenvManager implements EnvironmentManager { this.api, this.log, this, - scope ? [scope] : undefined, + scopeRoot !== undefined ? [Uri.file(scopeRoot)] : undefined, )) ?? []; - if (scope) { - return this.mergeScopedEnvironments(scope, discovered); + if (scopeRoot !== undefined) { + return this.mergeScopedEnvironments(scopeRoot, discovered); } const discard = this.collection.map((env) => ({ kind: EnvironmentChangeKind.remove, @@ -369,55 +377,64 @@ export class VenvManager implements EnvironmentManager { // folders (and globals outside it) are retained untouched, so they neither disappear nor emit // events; only in-scope environments are replaced by the freshly discovered ones. private async mergeScopedEnvironments( - scope: Uri, + scopeRoot: string, discovered: readonly PythonEnvironment[], ): Promise { - let scopeRoot: string; - try { - scopeRoot = await findParentIfFile(scope.fsPath); - } catch { - return undefined; - } const inScope = (fsPath: string): boolean => isPathInside(scopeRoot, fsPath); const retained: PythonEnvironment[] = []; - const removed: PythonEnvironment[] = []; + const oldInScope: PythonEnvironment[] = []; + const oldByPath = new Map(); for (const env of this.collection) { - (inScope(env.environmentPath.fsPath) ? removed : retained).push(env); + if (inScope(env.environmentPath.fsPath)) { + oldInScope.push(env); + const key = normalizePath(env.environmentPath.fsPath); + if (!oldByPath.has(key)) { + oldByPath.set(key, env); + } + } else { + retained.push(env); + } } - const seenPaths = new Set(retained.map((env) => normalizePath(env.environmentPath.fsPath))); - const added: PythonEnvironment[] = []; + + const retainedPaths = new Set(retained.map((env) => normalizePath(env.environmentPath.fsPath))); + const discoveredByPath = new Map(); for (const env of discovered) { if (!inScope(env.environmentPath.fsPath)) { continue; } const key = normalizePath(env.environmentPath.fsPath); - if (seenPaths.has(key)) { + if (retainedPaths.has(key) || discoveredByPath.has(key)) { continue; } - seenPaths.add(key); - added.push(env); + discoveredByPath.set(key, env); } - this.collection = [...retained, ...added]; + const finalInScope: PythonEnvironment[] = []; + const added: PythonEnvironment[] = []; + for (const [key, env] of discoveredByPath) { + if (oldByPath.get(key) === env) { + finalInScope.push(env); + } else { + added.push(env); + finalInScope.push(env); + } + } + + this.collection = [...retained, ...finalInScope]; const appended = await this.loadEnvMap(inScope); - const currentIds = new Set(this.collection.map((env) => env.envId.id)); - const seenAddIds = new Set(); - const addChanges: DidChangeEnvironmentsEventArgs = []; + const changes: DidChangeEnvironmentsEventArgs = []; + for (const env of oldInScope) { + if (!this.collection.includes(env)) { + changes.push({ environment: env, kind: EnvironmentChangeKind.remove }); + } + } for (const env of [...added, ...appended]) { - if (!this.collection.includes(env) || seenAddIds.has(env.envId.id)) { - continue; + if (this.collection.includes(env)) { + changes.push({ environment: env, kind: EnvironmentChangeKind.add }); } - seenAddIds.add(env.envId.id); - addChanges.push({ environment: env, kind: EnvironmentChangeKind.add }); } - const changes: DidChangeEnvironmentsEventArgs = [ - ...removed - .filter((env) => !currentIds.has(env.envId.id)) - .map((env) => ({ kind: EnvironmentChangeKind.remove, environment: env })), - ...addChanges, - ]; return changes.length > 0 ? changes : undefined; } diff --git a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts index a53ff2d83..bbfc12ea1 100644 --- a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts +++ b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts @@ -82,6 +82,75 @@ suite('VenvManager - scoped refresh preservation', () => { ); }); + test('replaces a same-path environment sharing its id when the discovered object differs', async () => { + const manager = createManager(); + const envAOld = makeEnv('A', venvARoot, '3.11.0'); + seed(manager, [envAOld]); + + const envANew = makeEnv('A', venvARoot, '3.12.5'); + findVirtualEnvironmentsStub.resolves([envANew]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + const collection: PythonEnvironment[] = (manager as any).collection; + assert.strictEqual(collection.length, 1); + assert.strictEqual(collection[0], envANew); + assert.ok(!collection.includes(envAOld)); + + const changes = flatChanges(events); + assert.deepStrictEqual( + changes.map((c) => ({ id: c.environment.envId.id, kind: c.kind })), + [ + { id: 'A', kind: EnvironmentChangeKind.remove }, + { id: 'A', kind: EnvironmentChangeKind.add }, + ], + ); + assert.strictEqual(changes[0].environment, envAOld); + assert.strictEqual(changes[1].environment, envANew); + }); + + test('emits no event when the exact same environment object is rediscovered in scope', async () => { + const manager = createManager(); + const envA = makeEnv('A', venvARoot); + seed(manager, [envA]); + + findVirtualEnvironmentsStub.resolves([envA]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + const collection: PythonEnvironment[] = (manager as any).collection; + assert.strictEqual(collection.length, 1); + assert.strictEqual(collection[0], envA); + assert.deepStrictEqual(events, []); + }); + + test('emits a remove for every stale in-scope duplicate that shares a normalized path', async () => { + const manager = createManager(); + const dup1 = makeEnv('A-dup-1', venvARoot); + const dup2 = makeEnv('A-dup-2', venvARoot); + seed(manager, [dup1, dup2, makeEnv('B', venvBRoot)]); + + findVirtualEnvironmentsStub.resolves([makeEnv('A-new', venvARoot)]); + + const events = captureEvents(manager); + await manager.refresh(Uri.file(folderA)); + + const collection: PythonEnvironment[] = (manager as any).collection; + assert.deepStrictEqual(ids(collection), ['A-new', 'B']); + assert.ok(!collection.includes(dup1) && !collection.includes(dup2)); + + const changes = flatChanges(events).map((c) => ({ id: c.environment.envId.id, kind: c.kind })); + const removed = changes + .filter((c) => c.kind === EnvironmentChangeKind.remove) + .map((c) => c.id) + .sort(); + const added = changes.filter((c) => c.kind === EnvironmentChangeKind.add).map((c) => c.id); + assert.deepStrictEqual(removed, ['A-dup-1', 'A-dup-2']); + assert.deepStrictEqual(added, ['A-new']); + }); + test('removes only stale environments inside the target scope', async () => { const manager = createManager(); seed(manager, [makeEnv('A', venvARoot), makeEnv('B', venvBRoot), makeEnv('G', globalVenvRoot)]); @@ -262,6 +331,7 @@ suite('VenvManager - scoped refresh preservation', () => { const events = captureEvents(manager); await manager.refresh(missingNested); + assert.ok(findVirtualEnvironmentsStub.notCalled); assert.deepStrictEqual(ids((manager as any).collection), ['OTHER', 'PKG']); assert.deepStrictEqual(events, []); }); @@ -291,6 +361,7 @@ suite('VenvManager - scoped refresh preservation', () => { const events = captureEvents(manager); await manager.refresh(Uri.file(path.join(folderA, 'deleted.py'))); + assert.ok(findVirtualEnvironmentsStub.notCalled); assert.deepStrictEqual(ids((manager as any).collection), ['A-old', 'B']); assert.deepStrictEqual(events, []); }); @@ -615,7 +686,24 @@ suite('VenvManager - scoped refresh preservation', () => { assert.strictEqual(uris[0].fsPath, scope.fsPath); }); - function createManager(): VenvManager { + test('resolves a file scope to its directory before invoking the native finder', async () => { + findVirtualEnvironmentsStub.restore(); + const finderRefresh = sinon.stub().resolves([]); + const manager = createManager({ refresh: finderRefresh } as unknown as NativePythonFinder); + seed(manager, []); + + const fileUri = Uri.file(path.join(folderA, 'main.py')); + findParentIfFileStub.callsFake(async () => folderA); + + await manager.refresh(fileUri); + + assert.ok(finderRefresh.calledOnce); + const uris = finderRefresh.firstCall.args[1] as Uri[] | undefined; + assert.ok(Array.isArray(uris) && uris.length === 1); + assert.strictEqual(uris[0].fsPath, Uri.file(folderA).fsPath); + }); + + function createManager(finder: NativePythonFinder = {} as NativePythonFinder): VenvManager { const api = { getEnvironments: sinon.stub().resolves([]), getPythonProject: sinon.stub().returns(undefined), @@ -625,7 +713,7 @@ suite('VenvManager - scoped refresh preservation', () => { const baseManager = { getEnvironments: sinon.stub().resolves([]), } as any as EnvironmentManager; - const manager = new VenvManager({} as NativePythonFinder, api, baseManager, { + const manager = new VenvManager(finder, api, baseManager, { info: sinon.stub(), error: sinon.stub(), warn: sinon.stub(), From bce080d2492217c0def0bf38615d45ff95b313cc Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Sun, 23 Aug 2026 20:30:09 -0700 Subject: [PATCH 5/9] fix: fail closed only on ENOENT scope inspection and guard stale discovery Restrict the scoped refresh scope-inspection catch to swallow only ENOENT (ambiguous missing scope) and rethrow EACCES/EPERM/unexpected I/O errors so refresh() rejects and surfaces the failure while the chain still recovers. Add a narrow direct-mutation generation counter bumped only by successful user/direct collection mutations (create add/replace, remove, set-global append). Capture it before discovery and discard the stale discovery result (no collection or event mutation) when a direct mutation changed the collection while discovery was in flight, for both scoped and full refreshes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/managers/builtin/venvManager.ts | 24 ++- .../venvManager.scopedRefresh.unit.test.ts | 164 +++++++++++++++++- 2 files changed, 182 insertions(+), 6 deletions(-) diff --git a/src/managers/builtin/venvManager.ts b/src/managers/builtin/venvManager.ts index ddb159b73..72aebfc64 100644 --- a/src/managers/builtin/venvManager.ts +++ b/src/managers/builtin/venvManager.ts @@ -52,6 +52,7 @@ import { export class VenvManager implements EnvironmentManager { private collection: PythonEnvironment[] = []; private refreshChain: Promise = Promise.resolve(); + private collectionMutationGeneration = 0; private readonly fsPathToEnv: Map = new Map(); private globalEnv: PythonEnvironment | undefined; private skipWatcherRefresh = false; @@ -287,7 +288,11 @@ export class VenvManager implements EnvironmentManager { private updateCollection(environment: PythonEnvironment): void { const envPath = normalizePath(environment.environmentPath.fsPath); + const before = this.collection.length; this.collection = this.collection.filter((e) => normalizePath(e.environmentPath.fsPath) !== envPath); + if (this.collection.length !== before) { + this.collectionMutationGeneration++; + } } private updateFsPathToEnv(environment: PythonEnvironment): Uri[] { @@ -327,12 +332,16 @@ export class VenvManager implements EnvironmentManager { async () => { const run = this.refreshChain.then( async (): Promise => { + const generation = this.collectionMutationGeneration; let scopeRoot: string | undefined; if (scope) { try { scopeRoot = await findParentIfFile(scope.fsPath); - } catch { - return undefined; + } catch (err) { + if ((err as NodeJS.ErrnoException)?.code === 'ENOENT') { + return undefined; + } + throw err; } } const discovered = @@ -344,6 +353,9 @@ export class VenvManager implements EnvironmentManager { this, scopeRoot !== undefined ? [Uri.file(scopeRoot)] : undefined, )) ?? []; + if (this.collectionMutationGeneration !== generation) { + return undefined; + } if (scopeRoot !== undefined) { return this.mergeScopedEnvironments(scopeRoot, discovered); } @@ -620,13 +632,19 @@ export class VenvManager implements EnvironmentManager { this._onDidChangeEnvironments.fire([{ environment, kind: EnvironmentChangeKind.add }]); } } + if (raiseEvent) { + this.collectionMutationGeneration++; + } return environment; } private async resetGlobalEnv() { this.globalEnv = undefined; const globals = await this.baseManager.getEnvironments('global'); - await this.loadGlobalEnv(globals); + const added = await this.loadGlobalEnv(globals); + if (added) { + this.collectionMutationGeneration++; + } } /** diff --git a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts index bbfc12ea1..073fcde30 100644 --- a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts +++ b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts @@ -366,7 +366,7 @@ suite('VenvManager - scoped refresh preservation', () => { assert.deepStrictEqual(events, []); }); - test('skips reconciliation without widening when a nested scope inspection fails with EACCES', async () => { + test('rejects and surfaces the error without discovery when a scope inspection fails with EACCES', async () => { const manager = createManager(); const pkgVenv = path.join(folderA, 'pkg', '.venv'); const otherVenv = path.join(folderA, 'other', '.venv'); @@ -374,8 +374,10 @@ suite('VenvManager - scoped refresh preservation', () => { const nestedScope = Uri.file(path.join(folderA, 'pkg')); ((manager as any).api.getPythonProject as sinon.SinonStub).returns({ uri: Uri.file(folderA) }); + let call = 0; findParentIfFileStub.callsFake(async (p: string) => { - if (p === nestedScope.fsPath) { + call += 1; + if (call === 1) { throw Object.assign(new Error('EACCES: permission denied'), { code: 'EACCES' }); } return p; @@ -383,10 +385,15 @@ suite('VenvManager - scoped refresh preservation', () => { findVirtualEnvironmentsStub.resolves([makeEnv('PKG-new', pkgVenv)]); const events = captureEvents(manager); - await manager.refresh(nestedScope); + await assert.rejects(manager.refresh(nestedScope), /EACCES/); + assert.ok(findVirtualEnvironmentsStub.notCalled); assert.deepStrictEqual(ids((manager as any).collection), ['OTHER', 'PKG']); assert.deepStrictEqual(events, []); + + await manager.refresh(nestedScope); + assert.ok(findVirtualEnvironmentsStub.called); + assert.deepStrictEqual(ids((manager as any).collection), ['OTHER', 'PKG-new']); }); test('does not append an out-of-scope environment while loading the project map', async () => { @@ -651,6 +658,157 @@ suite('VenvManager - scoped refresh preservation', () => { ); }); + test('discards a stale scoped discovery when a direct remove mutates the collection during discovery', async () => { + const manager = createManager(); + const envA = makeEnv('A', venvARoot); + seed(manager, [envA]); + sinon.stub(venvUtils, 'removeVenv').resolves(true); + + const inDiscovery = createDeferred(); + const releaseDiscovery = createDeferred(); + findVirtualEnvironmentsStub.callsFake(async () => { + inDiscovery.resolve(); + await releaseDiscovery.promise; + return [envA]; + }); + + const events = captureEvents(manager); + const pRefresh = manager.refresh(Uri.file(folderA)); + await inDiscovery.promise; + await manager.remove(envA); + releaseDiscovery.resolve(); + await pRefresh; + + assert.deepStrictEqual(ids((manager as any).collection), []); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [[{ id: 'A', kind: EnvironmentChangeKind.remove }]], + ); + }); + + test('discards a stale full refresh when a direct remove mutates the collection during discovery', async () => { + const manager = createManager(); + const envA = makeEnv('A', venvARoot); + seed(manager, [envA]); + sinon.stub(venvUtils, 'removeVenv').resolves(true); + + const inDiscovery = createDeferred(); + const releaseDiscovery = createDeferred(); + findVirtualEnvironmentsStub.callsFake(async () => { + inDiscovery.resolve(); + await releaseDiscovery.promise; + return [envA]; + }); + + const events = captureEvents(manager); + const pRefresh = manager.refresh(undefined); + await inDiscovery.promise; + await manager.remove(envA); + releaseDiscovery.resolve(); + await pRefresh; + + assert.deepStrictEqual(ids((manager as any).collection), []); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [[{ id: 'A', kind: EnvironmentChangeKind.remove }]], + ); + }); + + test('discards a stale scoped discovery when a direct create mutates the collection during discovery', async () => { + const manager = createManager(); + seed(manager, []); + + const envA = makeEnv('A', venvARoot); + const created = makeEnv('CREATED', path.join(ROOT, 'created', '.venv')); + const inDiscovery = createDeferred(); + const releaseDiscovery = createDeferred(); + findVirtualEnvironmentsStub.callsFake(async () => { + inDiscovery.resolve(); + await releaseDiscovery.promise; + return [envA]; + }); + + const events = captureEvents(manager); + const pRefresh = manager.refresh(Uri.file(folderA)); + await inDiscovery.promise; + (manager as any).addEnvironment(created, true); + releaseDiscovery.resolve(); + await pRefresh; + + assert.deepStrictEqual(ids((manager as any).collection), ['CREATED']); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [[{ id: 'CREATED', kind: EnvironmentChangeKind.add }]], + ); + }); + + test('resumes on a later refresh after discarding a stale discovery result', async () => { + const manager = createManager(); + const envA = makeEnv('A', venvARoot); + seed(manager, [envA]); + sinon.stub(venvUtils, 'removeVenv').resolves(true); + + const inDiscovery = createDeferred(); + const releaseDiscovery = createDeferred(); + let call = 0; + findVirtualEnvironmentsStub.callsFake(async () => { + call += 1; + if (call === 1) { + inDiscovery.resolve(); + await releaseDiscovery.promise; + return [envA]; + } + return [makeEnv('A-new', venvARoot)]; + }); + + const events = captureEvents(manager); + const pRefresh = manager.refresh(Uri.file(folderA)); + await inDiscovery.promise; + await manager.remove(envA); + releaseDiscovery.resolve(); + await pRefresh; + + await manager.refresh(Uri.file(folderA)); + + assert.deepStrictEqual(ids((manager as any).collection), ['A-new']); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [ + [{ id: 'A', kind: EnvironmentChangeKind.remove }], + [{ id: 'A-new', kind: EnvironmentChangeKind.add }], + ], + ); + }); + + test('discards a stale full refresh when setting the global env mutates the collection during discovery', async () => { + const manager = createManager(); + const envA = makeEnv('A', venvARoot); + seed(manager, [envA]); + + const globalEnv = makeEnv('G', globalVenvRoot); + sinon.stub(venvUtils, 'setVenvForGlobal').resolves(); + (venvUtils.getVenvForGlobal as sinon.SinonStub).resolves(venvPython(globalVenvRoot)); + (venvUtils.resolveVenvPythonEnvironmentPath as sinon.SinonStub).resolves(globalEnv); + + const inDiscovery = createDeferred(); + const releaseDiscovery = createDeferred(); + findVirtualEnvironmentsStub.callsFake(async () => { + inDiscovery.resolve(); + await releaseDiscovery.promise; + return [makeEnv('A-STALE', venvARoot)]; + }); + + const events = captureEvents(manager); + const pRefresh = manager.refresh(undefined); + await inDiscovery.promise; + await manager.set(undefined, globalEnv); + releaseDiscovery.resolve(); + await pRefresh; + + assert.deepStrictEqual(ids((manager as any).collection), ['A', 'G']); + assert.deepStrictEqual(events, []); + }); + test('full (unscoped) refresh still replaces the entire collection', async () => { const manager = createManager(); seed(manager, [makeEnv('A', venvARoot), makeEnv('B', venvBRoot)]); From f19ba54458412fb3f435860788ed99baae75ad3e Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Sun, 23 Aug 2026 21:13:18 -0700 Subject: [PATCH 6/9] fix: emit collection add event when set() resolves a new global venv A direct set(undefined, env) that resolves a previously undiscovered global environment appends it to the collection via loadGlobalEnv/addEnvironment (raiseEvent=false) and bumps the direct-mutation generation, which correctly discards any in-flight refresh as stale. But the append fired no onDidChangeEnvironments add event, so subscribers never learned about the new collection member and the discarded refresh could not publish it either. resetGlobalEnv now publishes exactly one add event for the environment loadGlobalEnv appended (the same object) and bumps the generation once. The refresh-owned loadEnvMap path still calls loadGlobalEnv directly and computes its own events, so it is unaffected and no add is double-emitted. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/managers/builtin/venvManager.ts | 1 + .../venvManager.scopedRefresh.unit.test.ts | 43 ++++++++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/managers/builtin/venvManager.ts b/src/managers/builtin/venvManager.ts index 72aebfc64..d80dcf095 100644 --- a/src/managers/builtin/venvManager.ts +++ b/src/managers/builtin/venvManager.ts @@ -643,6 +643,7 @@ export class VenvManager implements EnvironmentManager { const globals = await this.baseManager.getEnvironments('global'); const added = await this.loadGlobalEnv(globals); if (added) { + this._onDidChangeEnvironments.fire([{ environment: added, kind: EnvironmentChangeKind.add }]); this.collectionMutationGeneration++; } } diff --git a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts index 073fcde30..505d09299 100644 --- a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts +++ b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts @@ -780,7 +780,7 @@ suite('VenvManager - scoped refresh preservation', () => { ); }); - test('discards a stale full refresh when setting the global env mutates the collection during discovery', async () => { + test('emits the global add and discards a stale full refresh when setting the global env mutates the collection during discovery', async () => { const manager = createManager(); const envA = makeEnv('A', venvARoot); seed(manager, [envA]); @@ -806,7 +806,46 @@ suite('VenvManager - scoped refresh preservation', () => { await pRefresh; assert.deepStrictEqual(ids((manager as any).collection), ['A', 'G']); - assert.deepStrictEqual(events, []); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [[{ id: 'G', kind: EnvironmentChangeKind.add }]], + ); + }); + + test('selecting an already-collected global emits no add and does not discard a concurrent scoped refresh', async () => { + const manager = createManager(); + const envA = makeEnv('A', venvARoot); + const globalEnv = makeEnv('G', globalVenvRoot); + seed(manager, [envA, globalEnv]); + + sinon.stub(venvUtils, 'setVenvForGlobal').resolves(); + (venvUtils.getVenvForGlobal as sinon.SinonStub).resolves(venvPython(globalVenvRoot)); + + const inDiscovery = createDeferred(); + const releaseDiscovery = createDeferred(); + findVirtualEnvironmentsStub.callsFake(async () => { + inDiscovery.resolve(); + await releaseDiscovery.promise; + return [makeEnv('A-new', venvARoot)]; + }); + + const events = captureEvents(manager); + const pRefresh = manager.refresh(Uri.file(folderA)); + await inDiscovery.promise; + await manager.set(undefined, globalEnv); + releaseDiscovery.resolve(); + await pRefresh; + + assert.deepStrictEqual(ids((manager as any).collection), ['A-new', 'G']); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [ + [ + { id: 'A', kind: EnvironmentChangeKind.remove }, + { id: 'A-new', kind: EnvironmentChangeKind.add }, + ], + ], + ); }); test('full (unscoped) refresh still replaces the entire collection', async () => { From 52b8e7e9f99e7d86a938cdec770cdf2e0261e58d Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Sun, 23 Aug 2026 21:41:10 -0700 Subject: [PATCH 7/9] fix: derive full-refresh add events from operation-local discovery The full (unscoped) refresh replaced the collection and then computed its add events from the entire shared collection after awaiting loadEnvMap. A direct create (or set-global add) during that await pushes its own env and fires its own add, so the refresh re-emitted the same env as a duplicate add. Assign a copy of the discovered results to the collection (so the discovery snapshot stays pristine), capture loadEnvMap's appended return, and compute add events only from those operation-local sources revalidated against the current collection. This mirrors the scoped path and drops concurrently created/removed envs from the refresh's own event batch without changing the common-case behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/managers/builtin/venvManager.ts | 14 ++++--- .../venvManager.scopedRefresh.unit.test.ts | 38 +++++++++++++++++++ 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/src/managers/builtin/venvManager.ts b/src/managers/builtin/venvManager.ts index d80dcf095..cafb98499 100644 --- a/src/managers/builtin/venvManager.ts +++ b/src/managers/builtin/venvManager.ts @@ -363,12 +363,14 @@ export class VenvManager implements EnvironmentManager { kind: EnvironmentChangeKind.remove, environment: env, })); - this.collection = discovered; - await this.loadEnvMap(); - const added = this.collection.map((env) => ({ - environment: env, - kind: EnvironmentChangeKind.add, - })); + this.collection = [...discovered]; + const appended = await this.loadEnvMap(); + const added = [...discovered, ...appended] + .filter((env) => this.collection.includes(env)) + .map((env) => ({ + environment: env, + kind: EnvironmentChangeKind.add, + })); return [...discard, ...added]; }, ); diff --git a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts index 505d09299..892ca3eb2 100644 --- a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts +++ b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts @@ -587,6 +587,44 @@ suite('VenvManager - scoped refresh preservation', () => { ); }); + test('does not attribute a concurrent create to a full refresh while loading the project map', async () => { + const manager = createManager(); + seed(manager, [makeEnv('OLD', venvBRoot)]); + + findVirtualEnvironmentsStub.resolves([makeEnv('A', venvARoot)]); + + const inLoadEnvMap = createDeferred(); + const release = createDeferred(); + let call = 0; + ((manager as any).baseManager.getEnvironments as sinon.SinonStub).callsFake(async () => { + call += 1; + if (call === 1) { + inLoadEnvMap.resolve(); + await release.promise; + } + return []; + }); + + const events = captureEvents(manager); + const pRefresh = manager.refresh(undefined); + await inLoadEnvMap.promise; + (manager as any).addEnvironment(makeEnv('CREATED', path.join(ROOT, 'created', '.venv')), true); + release.resolve(); + await pRefresh; + + assert.deepStrictEqual(ids((manager as any).collection), ['A', 'CREATED']); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [ + [{ id: 'CREATED', kind: EnvironmentChangeKind.add }], + [ + { id: 'OLD', kind: EnvironmentChangeKind.remove }, + { id: 'A', kind: EnvironmentChangeKind.add }, + ], + ], + ); + }); + test('does not publish a stale add when a scoped refresh discovery is removed during project map loading', async () => { const manager = createManager(); seed(manager, []); From c851936531addcbf2bd4e9459d005ddfb499b40e Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Mon, 24 Aug 2026 07:47:28 -0700 Subject: [PATCH 8/9] fix: attribute concurrent direct removals so venv refresh does not duplicate remove events Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/managers/builtin/venvManager.ts | 37 ++++- .../venvManager.scopedRefresh.unit.test.ts | 153 ++++++++++++++++++ 2 files changed, 183 insertions(+), 7 deletions(-) diff --git a/src/managers/builtin/venvManager.ts b/src/managers/builtin/venvManager.ts index cafb98499..b2c69b2a8 100644 --- a/src/managers/builtin/venvManager.ts +++ b/src/managers/builtin/venvManager.ts @@ -53,6 +53,7 @@ export class VenvManager implements EnvironmentManager { private collection: PythonEnvironment[] = []; private refreshChain: Promise = Promise.resolve(); private collectionMutationGeneration = 0; + private readonly directRemovalGenerations = new Map(); private readonly fsPathToEnv: Map = new Map(); private globalEnv: PythonEnvironment | undefined; private skipWatcherRefresh = false; @@ -292,9 +293,15 @@ export class VenvManager implements EnvironmentManager { this.collection = this.collection.filter((e) => normalizePath(e.environmentPath.fsPath) !== envPath); if (this.collection.length !== before) { this.collectionMutationGeneration++; + this.directRemovalGenerations.set(envPath, this.collectionMutationGeneration); } } + private wasDirectlyRemovedSince(environment: PythonEnvironment, generation: number): boolean { + const removedGeneration = this.directRemovalGenerations.get(normalizePath(environment.environmentPath.fsPath)); + return removedGeneration !== undefined && removedGeneration > generation; + } + private updateFsPathToEnv(environment: PythonEnvironment): Uri[] { const envPath = normalizePath(environment.environmentPath.fsPath); const changed: Uri[] = []; @@ -333,6 +340,11 @@ export class VenvManager implements EnvironmentManager { const run = this.refreshChain.then( async (): Promise => { const generation = this.collectionMutationGeneration; + for (const [key, removedGeneration] of this.directRemovalGenerations) { + if (removedGeneration <= generation) { + this.directRemovalGenerations.delete(key); + } + } let scopeRoot: string | undefined; if (scope) { try { @@ -357,21 +369,25 @@ export class VenvManager implements EnvironmentManager { return undefined; } if (scopeRoot !== undefined) { - return this.mergeScopedEnvironments(scopeRoot, discovered); + return this.mergeScopedEnvironments(scopeRoot, discovered, generation); } - const discard = this.collection.map((env) => ({ - kind: EnvironmentChangeKind.remove, - environment: env, - })); + const previousCollection = this.collection; this.collection = [...discovered]; const appended = await this.loadEnvMap(); + const discard = previousCollection + .filter((env) => !this.wasDirectlyRemovedSince(env, generation)) + .map((env) => ({ + kind: EnvironmentChangeKind.remove, + environment: env, + })); const added = [...discovered, ...appended] .filter((env) => this.collection.includes(env)) .map((env) => ({ environment: env, kind: EnvironmentChangeKind.add, })); - return [...discard, ...added]; + const fullChanges = [...discard, ...added]; + return fullChanges.length > 0 ? fullChanges : undefined; }, ); this.refreshChain = run.then( @@ -393,6 +409,7 @@ export class VenvManager implements EnvironmentManager { private async mergeScopedEnvironments( scopeRoot: string, discovered: readonly PythonEnvironment[], + generation: number, ): Promise { const inScope = (fsPath: string): boolean => isPathInside(scopeRoot, fsPath); @@ -440,7 +457,7 @@ export class VenvManager implements EnvironmentManager { const changes: DidChangeEnvironmentsEventArgs = []; for (const env of oldInScope) { - if (!this.collection.includes(env)) { + if (!this.collection.includes(env) && !this.wasDirectlyRemovedSince(env, generation)) { changes.push({ environment: env, kind: EnvironmentChangeKind.remove }); } } @@ -636,6 +653,12 @@ export class VenvManager implements EnvironmentManager { } if (raiseEvent) { this.collectionMutationGeneration++; + if (oldEnv) { + this.directRemovalGenerations.set( + normalizePath(oldEnv.environmentPath.fsPath), + this.collectionMutationGeneration, + ); + } } return environment; } diff --git a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts index 892ca3eb2..0eb63bfb0 100644 --- a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts +++ b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts @@ -696,6 +696,159 @@ suite('VenvManager - scoped refresh preservation', () => { ); }); + test('does not duplicate a full-refresh remove when a direct remove completes during project map loading', async () => { + const manager = createManager(); + const envA = makeEnv('A', venvARoot); + seed(manager, [envA]); + sinon.stub(venvUtils, 'removeVenv').resolves(true); + + findVirtualEnvironmentsStub.resolves([envA]); + + const inLoadEnvMap = createDeferred(); + const release = createDeferred(); + let call = 0; + ((manager as any).baseManager.getEnvironments as sinon.SinonStub).callsFake(async () => { + call += 1; + if (call === 1) { + inLoadEnvMap.resolve(); + await release.promise; + } + return []; + }); + + const events = captureEvents(manager); + const pRefresh = manager.refresh(undefined); + await inLoadEnvMap.promise; + await manager.remove(envA); + release.resolve(); + await pRefresh; + + assert.deepStrictEqual(ids((manager as any).collection), []); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [[{ id: 'A', kind: EnvironmentChangeKind.remove }]], + ); + }); + + test('emits a directly removed env once while republishing surviving envs during a full refresh', async () => { + const manager = createManager(); + const envA = makeEnv('A', venvARoot); + const envB = makeEnv('B', venvBRoot); + seed(manager, [envA, envB]); + sinon.stub(venvUtils, 'removeVenv').resolves(true); + + findVirtualEnvironmentsStub.resolves([envA, envB]); + + const inLoadEnvMap = createDeferred(); + const release = createDeferred(); + let call = 0; + ((manager as any).baseManager.getEnvironments as sinon.SinonStub).callsFake(async () => { + call += 1; + if (call === 1) { + inLoadEnvMap.resolve(); + await release.promise; + } + return []; + }); + + const events = captureEvents(manager); + const pRefresh = manager.refresh(undefined); + await inLoadEnvMap.promise; + await manager.remove(envA); + release.resolve(); + await pRefresh; + + assert.deepStrictEqual(ids((manager as any).collection), ['B']); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [ + [{ id: 'A', kind: EnvironmentChangeKind.remove }], + [ + { id: 'B', kind: EnvironmentChangeKind.remove }, + { id: 'B', kind: EnvironmentChangeKind.add }, + ], + ], + ); + }); + + test('does not duplicate a replacement remove when a direct create replaces an env during full-refresh map loading', async () => { + const manager = createManager(); + const envAOld = makeEnv('A-old', venvARoot); + const envB = makeEnv('B', venvBRoot); + seed(manager, [envAOld, envB]); + + findVirtualEnvironmentsStub.resolves([envAOld, envB]); + + const inLoadEnvMap = createDeferred(); + const release = createDeferred(); + let call = 0; + ((manager as any).baseManager.getEnvironments as sinon.SinonStub).callsFake(async () => { + call += 1; + if (call === 1) { + inLoadEnvMap.resolve(); + await release.promise; + } + return []; + }); + + const envANew = makeEnv('A-new', venvARoot); + const events = captureEvents(manager); + const pRefresh = manager.refresh(undefined); + await inLoadEnvMap.promise; + (manager as any).addEnvironment(envANew, true); + release.resolve(); + await pRefresh; + + assert.deepStrictEqual(ids((manager as any).collection), ['A-new', 'B']); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [ + [ + { id: 'A-old', kind: EnvironmentChangeKind.remove }, + { id: 'A-new', kind: EnvironmentChangeKind.add }, + ], + [ + { id: 'B', kind: EnvironmentChangeKind.remove }, + { id: 'B', kind: EnvironmentChangeKind.add }, + ], + ], + ); + }); + + test('does not duplicate a scoped remove when a direct remove completes during project map loading', async () => { + const manager = createManager(); + const envA = makeEnv('A', venvARoot); + seed(manager, [envA]); + sinon.stub(venvUtils, 'removeVenv').resolves(true); + + findVirtualEnvironmentsStub.resolves([envA]); + + const inLoadEnvMap = createDeferred(); + const release = createDeferred(); + let call = 0; + ((manager as any).baseManager.getEnvironments as sinon.SinonStub).callsFake(async () => { + call += 1; + if (call === 1) { + inLoadEnvMap.resolve(); + await release.promise; + } + return []; + }); + + const events = captureEvents(manager); + const pRefresh = manager.refresh(Uri.file(folderA)); + await inLoadEnvMap.promise; + await manager.remove(envA); + release.resolve(); + await pRefresh; + + assert.deepStrictEqual(ids((manager as any).collection), []); + assert.deepStrictEqual( + events.map((batch) => batch.map((c) => ({ id: c.environment.envId.id, kind: c.kind }))), + [[{ id: 'A', kind: EnvironmentChangeKind.remove }]], + ); + }); + test('discards a stale scoped discovery when a direct remove mutates the collection during discovery', async () => { const manager = createManager(); const envA = makeEnv('A', venvARoot); From 0d8c3c329c372b85365677654a1545fb659b675b Mon Sep 17 00:00:00 2001 From: Stella Huang Date: Mon, 24 Aug 2026 08:28:47 -0700 Subject: [PATCH 9/9] fix: key venv direct-removal provenance by environment object identity Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/managers/builtin/venvManager.ts | 36 +++++---- .../venvManager.scopedRefresh.unit.test.ts | 76 +++++++++++++++++++ 2 files changed, 96 insertions(+), 16 deletions(-) diff --git a/src/managers/builtin/venvManager.ts b/src/managers/builtin/venvManager.ts index b2c69b2a8..1e8fb109e 100644 --- a/src/managers/builtin/venvManager.ts +++ b/src/managers/builtin/venvManager.ts @@ -53,7 +53,7 @@ export class VenvManager implements EnvironmentManager { private collection: PythonEnvironment[] = []; private refreshChain: Promise = Promise.resolve(); private collectionMutationGeneration = 0; - private readonly directRemovalGenerations = new Map(); + private readonly directRemovalGenerations = new WeakMap(); private readonly fsPathToEnv: Map = new Map(); private globalEnv: PythonEnvironment | undefined; private skipWatcherRefresh = false; @@ -289,16 +289,26 @@ export class VenvManager implements EnvironmentManager { private updateCollection(environment: PythonEnvironment): void { const envPath = normalizePath(environment.environmentPath.fsPath); - const before = this.collection.length; - this.collection = this.collection.filter((e) => normalizePath(e.environmentPath.fsPath) !== envPath); - if (this.collection.length !== before) { + const removed: PythonEnvironment[] = []; + const kept: PythonEnvironment[] = []; + for (const e of this.collection) { + if (normalizePath(e.environmentPath.fsPath) === envPath) { + removed.push(e); + } else { + kept.push(e); + } + } + if (removed.length > 0) { + this.collection = kept; this.collectionMutationGeneration++; - this.directRemovalGenerations.set(envPath, this.collectionMutationGeneration); + for (const env of removed) { + this.directRemovalGenerations.set(env, this.collectionMutationGeneration); + } } } private wasDirectlyRemovedSince(environment: PythonEnvironment, generation: number): boolean { - const removedGeneration = this.directRemovalGenerations.get(normalizePath(environment.environmentPath.fsPath)); + const removedGeneration = this.directRemovalGenerations.get(environment); return removedGeneration !== undefined && removedGeneration > generation; } @@ -340,11 +350,6 @@ export class VenvManager implements EnvironmentManager { const run = this.refreshChain.then( async (): Promise => { const generation = this.collectionMutationGeneration; - for (const [key, removedGeneration] of this.directRemovalGenerations) { - if (removedGeneration <= generation) { - this.directRemovalGenerations.delete(key); - } - } let scopeRoot: string | undefined; if (scope) { try { @@ -636,7 +641,9 @@ export class VenvManager implements EnvironmentManager { } const oldEnv = this.findEnvironmentByPath(environment.environmentPath.fsPath); + let replaced: PythonEnvironment[] = []; if (oldEnv) { + replaced = this.collection.filter((e) => e.envId.id === oldEnv.envId.id); this.collection = this.collection.filter((e) => e.envId.id !== oldEnv.envId.id); this.collection.push(environment); if (raiseEvent) { @@ -653,11 +660,8 @@ export class VenvManager implements EnvironmentManager { } if (raiseEvent) { this.collectionMutationGeneration++; - if (oldEnv) { - this.directRemovalGenerations.set( - normalizePath(oldEnv.environmentPath.fsPath), - this.collectionMutationGeneration, - ); + for (const env of replaced) { + this.directRemovalGenerations.set(env, this.collectionMutationGeneration); } } return environment; diff --git a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts index 0eb63bfb0..90e651b43 100644 --- a/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts +++ b/src/test/managers/builtin/venvManager.scopedRefresh.unit.test.ts @@ -849,6 +849,82 @@ suite('VenvManager - scoped refresh preservation', () => { ); }); + test('emits the refresh remove for a distinct old object when a direct remove targets a same-path replacement during full-refresh map loading', async () => { + const manager = createManager(); + const envOld = makeEnv('A', venvARoot); + seed(manager, [envOld]); + sinon.stub(venvUtils, 'removeVenv').resolves(true); + + const envDiscovered = makeEnv('A', venvARoot); + findVirtualEnvironmentsStub.resolves([envDiscovered]); + + const inLoadEnvMap = createDeferred(); + const release = createDeferred(); + let call = 0; + ((manager as any).baseManager.getEnvironments as sinon.SinonStub).callsFake(async () => { + call += 1; + if (call === 1) { + inLoadEnvMap.resolve(); + await release.promise; + } + return []; + }); + + const events = captureEvents(manager); + const pRefresh = manager.refresh(undefined); + await inLoadEnvMap.promise; + await manager.remove(envDiscovered); + release.resolve(); + await pRefresh; + + assert.deepStrictEqual(ids((manager as any).collection), []); + assert.strictEqual(events.length, 2); + assert.strictEqual(events[0].length, 1); + assert.strictEqual(events[0][0].kind, EnvironmentChangeKind.remove); + assert.strictEqual(events[0][0].environment, envDiscovered); + assert.strictEqual(events[1].length, 1); + assert.strictEqual(events[1][0].kind, EnvironmentChangeKind.remove); + assert.strictEqual(events[1][0].environment, envOld); + }); + + test('emits the refresh remove for a distinct old object when a direct remove targets a same-path scoped replacement during map loading', async () => { + const manager = createManager(); + const envOld = makeEnv('A', venvARoot); + seed(manager, [envOld]); + sinon.stub(venvUtils, 'removeVenv').resolves(true); + + const envDiscovered = makeEnv('A', venvARoot); + findVirtualEnvironmentsStub.resolves([envDiscovered]); + + const inLoadEnvMap = createDeferred(); + const release = createDeferred(); + let call = 0; + ((manager as any).baseManager.getEnvironments as sinon.SinonStub).callsFake(async () => { + call += 1; + if (call === 1) { + inLoadEnvMap.resolve(); + await release.promise; + } + return []; + }); + + const events = captureEvents(manager); + const pRefresh = manager.refresh(Uri.file(folderA)); + await inLoadEnvMap.promise; + await manager.remove(envDiscovered); + release.resolve(); + await pRefresh; + + assert.deepStrictEqual(ids((manager as any).collection), []); + assert.strictEqual(events.length, 2); + assert.strictEqual(events[0].length, 1); + assert.strictEqual(events[0][0].kind, EnvironmentChangeKind.remove); + assert.strictEqual(events[0][0].environment, envDiscovered); + assert.strictEqual(events[1].length, 1); + assert.strictEqual(events[1][0].kind, EnvironmentChangeKind.remove); + assert.strictEqual(events[1][0].environment, envOld); + }); + test('discards a stale scoped discovery when a direct remove mutates the collection during discovery', async () => { const manager = createManager(); const envA = makeEnv('A', venvARoot);