Skip to content

Fire onDidChangeEnvironment for global and single-Uri scopes in set()#1458

Open
StellaHuang95 wants to merge 1 commit intomicrosoft:mainfrom
StellaHuang95:2-fix-missing-onDidChangeEnvironment-events
Open

Fire onDidChangeEnvironment for global and single-Uri scopes in set()#1458
StellaHuang95 wants to merge 1 commit intomicrosoft:mainfrom
StellaHuang95:2-fix-missing-onDidChangeEnvironment-events

Conversation

@StellaHuang95
Copy link
Copy Markdown
Contributor

@StellaHuang95 StellaHuang95 commented Apr 14, 2026

Part of #1454

Bug

CondaEnvManager.set() only fires _onDidChangeEnvironment in the Uri[] (multi-project) branch. The scope === undefined (global) and scope instanceof Uri (single project) branches persist the selection and update internal maps, but never fire the change event.

The venvManager.ts correctly fires the event in all three branches (lines 413, 444, 477), so this is an inconsistency specific to the conda manager.

Why it's a bug

There are code paths that call manager.set() directly, bypassing the orchestrator's setEnvironment():

  1. envCommands.ts:220 — After creating a conda environment in the "global" (no workspace) context, calls manager.set(undefined, env) directly.
  2. extension.ts:344 — When removing a project, calls manager.set(uri, undefined) directly.
  3. envManagers.ts:404 and :434 — Batch operations that call manager.set() directly for individual items.

When these code paths run, the orchestrator's setEnvironment() is never involved, so it never fires its own _onDidChangeEnvironmentFiltered event. The only way listeners can be notified is through the manager-level _onDidChangeEnvironment — which conda doesn't fire.

Event chain that breaks

manager.set(undefined, env)              // called directly, not through orchestrator
  → conda persists to disk ✅
  → conda updates in-memory state ✅
  → conda fires _onDidChangeEnvironment ❌  (BUG)

envManagers._onDidChangeEnvironment      // relayed from manager event → never fires
  → extension.ts:511 updateViewsAndStatus  ❌
  → projectView.ts:62 tree refresh        ❌

pythonApi._onDidChangeEnvironment        // driven by _onDidChangeEnvironmentFiltered → never fires
  → shellStartupActivationVariablesManager ❌  (terminal activation not updated)
  → 3rd party extensions                   ❌

What the orchestrator masks

When the user selects an environment through the normal UI picker, the orchestrator's setEnvironment() calls manager.set() and then fires _onDidChangeEnvironmentFiltered independently. This masks the missing manager event for that specific flow. But the direct-call paths above are not masked.

Repro steps

Primary repro (global create flow):

  1. Open VS Code with no workspace folder open.
  2. Run "Python: Create Environment" from the command palette.
  3. Select Conda, choose a name and Python version.
  4. The environment is created successfully and manager.set(undefined, env) is called at envCommands.ts:220.
  5. Bug: The status bar still shows the old/no environment. The project view doesn't update. Terminal activation doesn't change. The UI appears frozen even though the environment was created and set internally.

Secondary repro (project removal):

  1. Open a multi-root workspace with a Python project using a conda environment.
  2. Remove the project from the workspace.
  3. manager.set(uri, undefined) is called at extension.ts:344 to clear the environment.
  4. Bug: Listeners don't know the environment was cleared — stale state remains in views and terminal activation.

Fix

  1. Global scope (scope === undefined): Capture the "before" state, update this.globalEnv, persist, then fire _onDidChangeEnvironment if the environment actually changed.
  2. Single Uri scope (scope instanceof Uri): Capture the "before" state from the map before updating, then fire _onDidChangeEnvironment if the environment actually changed.

Both branches now follow the same pattern as the Uri[] branch and the venvManager.ts implementation.

Before:
https://github.com/user-attachments/assets/5ec01adc-caeb-4db6-9620-deb0662b92f8

After:
https://github.com/user-attachments/assets/08baeceb-e9b7-45d4-9e5c-83792641fda6

Why this fix is correct

  • Events are only fired when the environment actually changes (before?.envId.id !== checkedEnv?.envId.id), preventing spurious notifications.
  • The in-memory globalEnv update ensures get(undefined) returns the correct value immediately after set().
  • No change to the Uri[] branch, which was already working correctly.
  • The change is purely additive — existing behavior is preserved, missing behavior is added.
  • For the normal picker flow (through the orchestrator), the manager event is now fired in addition to the orchestrator's event. This is consistent with venvManager and harmless — listeners that receive both just get a redundant but correct notification.

Tests added

  • condaEnvManager.setEvents.unit.test.ts: 5 test cases covering:
    • Global scope fires event
    • Global scope does not fire event when unchanged
    • Single Uri scope fires event
    • Single Uri scope does not fire event when unchanged
    • Clearing an environment (set to undefined) fires event

@StellaHuang95 StellaHuang95 added the bug Issue identified by VS Code Team member as probable bug label Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue identified by VS Code Team member as probable bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant