Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -508,13 +508,13 @@ export async function activate(context: ExtensionContext): Promise<PythonEnviron
window.onDidChangeActiveTextEditor(async () => {
updateViewsAndStatus(statusBar, workspaceView, managerView, api);
}),
envManagers.onDidChangeEnvironment(async () => {
envManagers.onDidChangeManagerEnvironment(async () => {
updateViewsAndStatus(statusBar, workspaceView, managerView, api);
}),
envManagers.onDidChangeEnvironments(async () => {
updateViewsAndStatus(statusBar, workspaceView, managerView, api);
}),
envManagers.onDidChangeEnvironmentFiltered(async (e) => {
envManagers.onDidChangeActiveEnvironment(async (e) => {
managerView.environmentChanged(e);
const location = e.uri?.fsPath ?? 'global';
traceInfo(
Expand Down
96 changes: 66 additions & 30 deletions src/features/envManagers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,23 +51,42 @@ function generateId(name: string, extensionId?: string): string {
export class PythonEnvironmentManagers implements EnvironmentManagers {
private _environmentManagers: Map<string, InternalEnvironmentManager> = new Map();
private _packageManagers: Map<string, InternalPackageManager> = new Map();
private readonly _previousEnvironments = new Map<string, PythonEnvironment | undefined>();

/**
* The last environment announced as "active" for each scope.
* Keyed by project URI string or 'global'.
*
* Used for: (1) change detection before firing onDidChangeActiveEnvironment,
* (2) fallback manager routing when no explicit setting exists.
*
* Only mutated by setEnvironment() / setEnvironments() / refreshEnvironment().
*/
private readonly _activeSelection = new Map<string, PythonEnvironment | undefined>();

private _onDidChangeEnvironmentManager = new EventEmitter<DidChangeEnvironmentManagerEventArgs>();
private _onDidChangePackageManager = new EventEmitter<DidChangePackageManagerEventArgs>();
private _onDidChangeEnvironments = new EventEmitter<InternalDidChangeEnvironmentsEventArgs>();
private _onDidChangeEnvironment = new EventEmitter<DidChangeEnvironmentEventArgs>();
private _onDidChangeEnvironmentFiltered = new EventEmitter<DidChangeEnvironmentEventArgs>();

/** Fires when ANY manager reports a selection change, regardless of whether that manager is selected. */
private _onDidChangeManagerEnvironment = new EventEmitter<DidChangeEnvironmentEventArgs>();

/** Fires when the active (selected) environment for a scope actually changes. */
private _onDidChangeActiveEnvironment = new EventEmitter<DidChangeEnvironmentEventArgs>();
private _onDidChangePackages = new EventEmitter<InternalDidChangePackagesEventArgs>();
Comment on lines +70 to 75
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

The new EventEmitters _onDidChangeManagerEnvironment / _onDidChangeActiveEnvironment are created here, but dispose() only disposes _onDidChangeEnvironmentManager, _onDidChangePackageManager, _onDidChangeEnvironments, and _onDidChangePackages. Please also dispose these new emitters (and any corresponding public events) to avoid leaking listeners when PythonEnvironmentManagers is disposed.

Copilot uses AI. Check for mistakes.

public onDidChangeEnvironmentManager: Event<DidChangeEnvironmentManagerEventArgs> =
this._onDidChangeEnvironmentManager.event;
public onDidChangePackageManager: Event<DidChangePackageManagerEventArgs> = this._onDidChangePackageManager.event;
public onDidChangeEnvironments: Event<InternalDidChangeEnvironmentsEventArgs> = this._onDidChangeEnvironments.event;
public onDidChangeEnvironment: Event<DidChangeEnvironmentEventArgs> = this._onDidChangeEnvironment.event;

/** Fires when any registered manager reports a change — even if that manager is not the selected one. */
public onDidChangeManagerEnvironment: Event<DidChangeEnvironmentEventArgs> =
this._onDidChangeManagerEnvironment.event;
public onDidChangePackages: Event<InternalDidChangePackagesEventArgs> = this._onDidChangePackages.event;
public onDidChangeEnvironmentFiltered: Event<DidChangeEnvironmentEventArgs> =
this._onDidChangeEnvironmentFiltered.event;

/** Fires only when the *selected* manager's environment for a scope actually changes. */
public onDidChangeActiveEnvironment: Event<DidChangeEnvironmentEventArgs> =
this._onDidChangeActiveEnvironment.event;

constructor(private readonly pm: PythonProjectManager) {}

Expand Down Expand Up @@ -99,7 +118,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
return;
}

setImmediate(() => this._onDidChangeEnvironment.fire(e));
setImmediate(() => this._onDidChangeManagerEnvironment.fire(e));
}),
);

Expand Down Expand Up @@ -165,6 +184,8 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
this._onDidChangeEnvironmentManager.dispose();
this._onDidChangePackageManager.dispose();
this._onDidChangeEnvironments.dispose();
this._onDidChangeManagerEnvironment.dispose();
this._onDidChangeActiveEnvironment.dispose();
this._onDidChangePackages.dispose();
}

Expand Down Expand Up @@ -196,7 +217,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
// Fall back to cached environment's manager if no user-configured settings
const project = context ? this.pm.get(context) : undefined;
const key = project ? project.uri.toString() : 'global';
const cachedEnv = this._previousEnvironments.get(key);
const cachedEnv = this._activeSelection.get(key);
if (cachedEnv) {
const cachedManager = this._environmentManagers.get(cachedEnv.envId.managerId);
if (cachedManager) {
Expand Down Expand Up @@ -339,11 +360,11 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
}

const key = project ? project.uri.toString() : 'global';
const oldEnv = this._previousEnvironments.get(key);
const oldEnv = this._activeSelection.get(key);
if (oldEnv?.envId.id !== environment?.envId.id) {
this._previousEnvironments.set(key, environment);
this._activeSelection.set(key, environment);
setImmediate(() =>
this._onDidChangeEnvironmentFiltered.fire({ uri: project?.uri, new: environment, old: oldEnv }),
this._onDidChangeActiveEnvironment.fire({ uri: project?.uri, new: environment, old: oldEnv }),
);
}
}
Expand Down Expand Up @@ -393,9 +414,9 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {

const project = this.pm.get(uri);
const key = project ? project.uri.toString() : 'global';
const oldEnv = this._previousEnvironments.get(key);
const oldEnv = this._activeSelection.get(key);
if (oldEnv?.envId.id !== environment?.envId.id) {
this._previousEnvironments.set(key, environment);
this._activeSelection.set(key, environment);
events.push({ uri: project?.uri, new: environment, old: oldEnv });
}
});
Expand All @@ -411,9 +432,9 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
});
}

const oldEnv = this._previousEnvironments.get('global');
const oldEnv = this._activeSelection.get('global');
if (oldEnv?.envId.id !== environment?.envId.id) {
this._previousEnvironments.set('global', environment);
this._activeSelection.set('global', environment);
events.push({ uri: undefined, new: environment, old: oldEnv });
}
}
Expand All @@ -422,7 +443,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
if (shouldPersistSettings) {
await setAllManagerSettings(settings);
}
setImmediate(() => events.forEach((e) => this._onDidChangeEnvironmentFiltered.fire(e)));
setImmediate(() => events.forEach((e) => this._onDidChangeActiveEnvironment.fire(e)));
} else {
const promises: Promise<void>[] = [];
const events: DidChangeEnvironmentEventArgs[] = [];
Expand All @@ -439,9 +460,9 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
// events. But it ensures that we always get the latest environment at the time of this call.
const newEnv = await manager.get(uri);
const key = project ? project.uri.toString() : 'global';
const oldEnv = this._previousEnvironments.get(key);
const oldEnv = this._activeSelection.get(key);
if (oldEnv?.envId.id !== newEnv?.envId.id) {
this._previousEnvironments.set(key, newEnv);
this._activeSelection.set(key, newEnv);
events.push({ uri: project?.uri, new: newEnv, old: oldEnv });
}
};
Expand All @@ -457,17 +478,17 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
// Always get the new first, then compare with the old. This has minor impact on the ordering of
// events. But it ensures that we always get the latest environment at the time of this call.
const newEnv = await manager.get(undefined);
const oldEnv = this._previousEnvironments.get('global');
const oldEnv = this._activeSelection.get('global');
if (oldEnv?.envId.id !== newEnv?.envId.id) {
this._previousEnvironments.set('global', newEnv);
this._activeSelection.set('global', newEnv);
events.push({ uri: undefined, new: newEnv, old: oldEnv });
}
};
promises.push(setAndAddEvent());
}
}
await Promise.all(promises);
setImmediate(() => events.forEach((e) => this._onDidChangeEnvironmentFiltered.fire(e)));
setImmediate(() => events.forEach((e) => this._onDidChangeActiveEnvironment.fire(e)));
}
}

Expand Down Expand Up @@ -503,8 +524,9 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
/**
* Gets the current Python environment for the given scope URI or undefined for 'global'.
*
* This method queries the appropriate environment manager for the latest environment for the scope.
* It also updates the internal cache and fires an event if the environment has changed since last check.
* This is a pure read: it queries the environment manager but does NOT update the
* internal selection cache or fire change events. Selection state is only mutated
* through setEnvironment() / setEnvironments() / refreshEnvironment().
*
* @param scope The scope to get the environment.
* @returns The current PythonEnvironment for the scope, or undefined if none is set.
Expand All @@ -519,21 +541,35 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
return undefined;
}

const project = scope ? this.pm.get(scope) : undefined;
return manager.get(scope);
}

/**
* Refreshes the cached environment for the given scope by querying the selected manager.
* If the manager's current environment differs from the cache, updates the cache and
* fires onDidChangeActiveEnvironment. Use this when a manager signals that its
* selection has changed (e.g., via onDidChangeManagerEnvironment).
*
* Unlike getEnvironment(), this IS a mutation — it updates internal state.
* Unlike setEnvironment(), it does NOT call manager.set() or persist to settings.
*/
async refreshEnvironment(scope: GetEnvironmentScope): Promise<void> {
const manager = this.getEnvironmentManager(scope);
if (!manager) {
return;
}

// Always get the new first, then compare with the old. This has minor impact on the ordering of
// events. But it ensures that we always get the latest environment at the time of this call.
const project = scope ? this.pm.get(scope) : undefined;
const newEnv = await manager.get(scope);

const key = project ? project.uri.toString() : 'global';
const oldEnv = this._previousEnvironments.get(key);
const oldEnv = this._activeSelection.get(key);
if (oldEnv?.envId.id !== newEnv?.envId.id) {
this._previousEnvironments.set(key, newEnv);
this._activeSelection.set(key, newEnv);
setImmediate(() =>
this._onDidChangeEnvironmentFiltered.fire({ uri: project?.uri, new: newEnv, old: oldEnv }),
this._onDidChangeActiveEnvironment.fire({ uri: project?.uri, new: newEnv, old: oldEnv }),
);
}
return newEnv;
}

getProjectEnvManagers(uris: Uri[]): InternalEnvironmentManager[] {
Expand Down
76 changes: 38 additions & 38 deletions src/features/pythonApi.ts
Original file line number Diff line number Diff line change
@@ -1,35 +1,40 @@
import { Uri, Disposable, Event, EventEmitter, Terminal, TaskExecution } from 'vscode';
import { Disposable, Event, EventEmitter, TaskExecution, Terminal, Uri } from 'vscode';
import {
PythonEnvironmentApi,
PythonEnvironment,
EnvironmentManager,
PackageManager,
CreateEnvironmentOptions,
CreateEnvironmentScope,
DidChangeEnvironmentEventArgs,
DidChangeEnvironmentsEventArgs,
DidChangeEnvironmentVariablesEventArgs,
DidChangePackagesEventArgs,
DidChangePythonProjectsEventArgs,
EnvironmentManager,
GetEnvironmentScope,
GetEnvironmentsScope,
Package,
PythonEnvironmentInfo,
PythonProject,
RefreshEnvironmentsScope,
DidChangePackagesEventArgs,
PythonEnvironmentId,
CreateEnvironmentScope,
SetEnvironmentScope,
GetEnvironmentScope,
PackageInfo,
PackageId,
PythonProjectCreator,
ResolveEnvironmentContext,
PackageInfo,
PackageManagementOptions,
PackageManager,
PythonBackgroundRunOptions,
PythonEnvironment,
PythonEnvironmentApi,
PythonEnvironmentId,
PythonEnvironmentInfo,
PythonProcess,
PythonProject,
PythonProjectCreator,
PythonTaskExecutionOptions,
PythonTerminalExecutionOptions,
PythonBackgroundRunOptions,
PythonTerminalCreateOptions,
DidChangeEnvironmentVariablesEventArgs,
CreateEnvironmentOptions,
PythonTerminalExecutionOptions,
RefreshEnvironmentsScope,
ResolveEnvironmentContext,
SetEnvironmentScope,
} from '../api';
import { traceError, traceInfo } from '../common/logging';
import { pickEnvironmentManager } from '../common/pickers/managers';
import { createDeferred } from '../common/utils/deferred';
import { checkUri } from '../common/utils/pathUtils';
import { handlePythonPath } from '../common/utils/pythonPath';
import {
EnvironmentManagers,
InternalEnvironmentManager,
Expand All @@ -38,17 +43,12 @@ import {
PythonPackageImpl,
PythonProjectManager,
} from '../internal.api';
import { createDeferred } from '../common/utils/deferred';
import { traceInfo } from '../common/logging';
import { pickEnvironmentManager } from '../common/pickers/managers';
import { handlePythonPath } from '../common/utils/pythonPath';
import { TerminalManager } from './terminal/terminalManager';
import { waitForAllEnvManagers, waitForEnvManager, waitForEnvManagerId } from './common/managerReady';
import { EnvVarManager } from './execution/envVariableManager';
import { runAsTask } from './execution/runAsTask';
import { runInTerminal } from './terminal/runInTerminal';
import { runInBackground } from './execution/runInBackground';
import { EnvVarManager } from './execution/envVariableManager';
import { checkUri } from '../common/utils/pathUtils';
import { waitForAllEnvManagers, waitForEnvManager, waitForEnvManagerId } from './common/managerReady';
import { runInTerminal } from './terminal/runInTerminal';
import { TerminalManager } from './terminal/terminalManager';

class PythonEnvironmentApiImpl implements PythonEnvironmentApi {
private readonly _onDidChangeEnvironments = new EventEmitter<DidChangeEnvironmentsEventArgs>();
Expand All @@ -71,7 +71,7 @@ class PythonEnvironmentApiImpl implements PythonEnvironmentApi {
this._onDidChangePythonProjects,
this._onDidChangePackages,
this._onDidChangeEnvironmentVariables,
this.envManagers.onDidChangeEnvironmentFiltered((e) => {
this.envManagers.onDidChangeActiveEnvironment((e) => {
this._onDidChangeEnvironment.fire(e);
const location = e.uri?.fsPath ?? 'global';
traceInfo(
Expand All @@ -91,14 +91,14 @@ class PythonEnvironmentApiImpl implements PythonEnvironmentApi {
if (manager.onDidChangeEnvironment) {
disposables.push(
manager.onDidChangeEnvironment((e) => {
setImmediate(async () => {
// This will ensure that we use the right manager and only trigger the event
// if the user selected manager decided to change the environment.
// This ensures that if a unselected manager changes environment and raises events
// we don't trigger the Python API event which can cause issues with the consumers.
// This will trigger onDidChangeEnvironmentFiltered event in envManagers, which the Python
// API listens to, and re-triggers the onDidChangeEnvironment event.
await this.envManagers.getEnvironment(e.uri);
setImmediate(() => {
// Refresh the central cache for this scope. This ensures that only the
// *selected* manager's changes propagate (refreshEnvironment checks
// getEnvironmentManager(scope) internally). It updates the cache and
// fires onDidChangeActiveEnvironment, which the Python API listens to.
this.envManagers.refreshEnvironment(e.uri).catch((err) =>
traceError('Failed to refresh environment on change:', err),
);
});
Comment on lines 93 to 102
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

The setImmediate(async () => ...) callback can throw/reject and will become an unhandled promise rejection (nothing awaits it). Wrap the async body in a try/catch (and log via traceError) or use void this.envManagers.refreshEnvironment(e.uri).catch(...) to ensure failures are surfaced without crashing the extension host.

Copilot uses AI. Check for mistakes.
}),
);
Expand Down
2 changes: 1 addition & 1 deletion src/features/views/projectView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export class ProjectView implements TreeDataProvider<ProjectTreeItem> {
this.projectManager.onDidChangeProjects(() => {
this.debouncedUpdateProject.trigger();
}),
this.envManagers.onDidChangeEnvironment(() => {
this.envManagers.onDidChangeManagerEnvironment(() => {
this.debouncedUpdateProject.trigger();
}),
this.envManagers.onDidChangeEnvironments(() => {
Expand Down
Loading
Loading