fix: use case-insensitive manager ID matching to prevent false install prompts

edvilme · Copilot · edvilme · commit 804eebe7a08f · 2026-04-29T12:53:23.000-07:00
The managerReady module used a case-sensitive startsWith scan against allExtensions() to check if an extension was installed. If there was any casing mismatch between the extension ID in settings and the ID returned by VS Code, the check would incorrectly report the extension as missing and prompt the user to install it. Additionally, the deferred maps used raw manager IDs as keys, so a registration event with different casing than the settings-provided ID would fail to resolve the waiting deferred. Changes: - Replace allExtensions().some() with getExtension() which is case-insensitive in VS Code's API - Move getExtension() call inside setImmediate to avoid stale state during startup - Normalize all map keys and checked set to lowercase - Add _resetManagerReadyForTesting() helper for test isolation - Add 5 regression tests covering case-insensitive matching Fixes #1465 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
diff --git a/src/features/common/managerReady.ts b/src/features/common/managerReady.ts
@@ -1,5 +1,5 @@
 import { Disposable, l10n, Uri } from 'vscode';
-import { allExtensions, getExtension } from '../../common/extension.apis';
+import { getExtension } from '../../common/extension.apis';
 import { WorkbenchStrings } from '../../common/localize';
 import { traceError, traceInfo, traceWarn } from '../../common/logging';
 import { EventNames } from '../../common/telemetry/constants';
@@ -81,38 +81,40 @@ class ManagerReadyImpl implements ManagerReady {
     ) {
         this.disposables.push(
             em.onDidChangeEnvironmentManager((e) => {
-                if (this.envManagers.has(e.manager.id)) {
-                    this.envManagers.get(e.manager.id)?.resolve();
+                const key = e.manager.id.toLowerCase();
+                if (this.envManagers.has(key)) {
+                    this.envManagers.get(key)?.resolve();
                 } else {
                     const deferred = createDeferred<void>();
-                    this.envManagers.set(e.manager.id, deferred);
+                    this.envManagers.set(key, deferred);
                     deferred.resolve();
                 }
             }),
             em.onDidChangePackageManager((e) => {
-                if (this.pkgManagers.has(e.manager.id)) {
-                    this.pkgManagers.get(e.manager.id)?.resolve();
+                const key = e.manager.id.toLowerCase();
+                if (this.pkgManagers.has(key)) {
+                    this.pkgManagers.get(key)?.resolve();
                 } else {
                     const deferred = createDeferred<void>();
-                    this.pkgManagers.set(e.manager.id, deferred);
+                    this.pkgManagers.set(key, deferred);
                     deferred.resolve();
                 }
             }),
         );
     }
 
     private checkExtension(managerId: string) {
-        const installed = allExtensions().some((ext) => managerId.startsWith(`${ext.id}:`));
-        if (this.checked.has(managerId)) {
+        const normalizedId = managerId.toLowerCase();
+        if (this.checked.has(normalizedId)) {
             return;
         }
-        this.checked.add(managerId);
+        this.checked.add(normalizedId);
         const extId = getExtensionId(managerId);
         if (extId) {
             setImmediate(async () => {
-                if (installed) {
-                    const ext = getExtension(extId);
-                    if (ext && !ext.isActive) {
+                const ext = getExtension(extId);
+                if (ext) {
+                    if (!ext.isActive) {
                         traceInfo(`Extension for manager ${managerId} is not active: Activating...`);
                         try {
                             await ext.activate();
@@ -122,7 +124,10 @@ class ManagerReadyImpl implements ManagerReady {
                         }
                     }
                 } else {
-                    traceError(`Extension for manager ${managerId} is not installed.`);
+                    traceError(
+                        `Extension for manager ${managerId} is not installed. ` +
+                            `Looked up extId="${extId}" via getExtension().`,
+                    );
                     const result = await showErrorMessage(
                         l10n.t(`Do you want to install extension {0} to enable {1} support.`, extId, managerId),
                         WorkbenchStrings.installExtension,
@@ -160,11 +165,12 @@ class ManagerReadyImpl implements ManagerReady {
     }
 
     private _waitForEnvManager(managerId: string): Promise<void> {
-        if (this.envManagers.has(managerId)) {
-            return this.envManagers.get(managerId)!.promise;
+        const key = managerId.toLowerCase();
+        if (this.envManagers.has(key)) {
+            return this.envManagers.get(key)!.promise;
         }
         const deferred = createDeferred<void>();
-        this.envManagers.set(managerId, deferred);
+        this.envManagers.set(key, deferred);
         return withManagerTimeout(deferred, managerId, 'environment');
     }
 
@@ -209,11 +215,12 @@ class ManagerReadyImpl implements ManagerReady {
     }
 
     private _waitForPkgManager(managerId: string): Promise<void> {
-        if (this.pkgManagers.has(managerId)) {
-            return this.pkgManagers.get(managerId)!.promise;
+        const key = managerId.toLowerCase();
+        if (this.pkgManagers.has(key)) {
+            return this.pkgManagers.get(key)!.promise;
         }
         const deferred = createDeferred<void>();
-        this.pkgManagers.set(managerId, deferred);
+        this.pkgManagers.set(key, deferred);
         return withManagerTimeout(deferred, managerId, 'package');
     }
 
@@ -251,6 +258,14 @@ export function createManagerReady(em: EnvironmentManagers, pm: PythonProjectMan
     }
 }
 
+/**
+ * Reset the module-level singleton so `createManagerReady` can be called again.
+ * Only for use in tests.
+ */
+export function _resetManagerReadyForTesting(): void {
+    _deferred = createDeferred<ManagerReady>();
+}
+
 export async function waitForEnvManager(uris?: Uri[]): Promise<void> {
     const mr = await _deferred.promise;
     return mr.waitForEnvManager(uris);
diff --git a/src/test/features/common/managerReady.unit.test.ts b/src/test/features/common/managerReady.unit.test.ts
@@ -1,10 +1,29 @@
 import assert from 'assert';
 import * as sinon from 'sinon';
+import { Disposable, EventEmitter } from 'vscode';
+import * as extensionApis from '../../../common/extension.apis';
 import * as logging from '../../../common/logging';
 import { EventNames } from '../../../common/telemetry/constants';
 import * as telemetrySender from '../../../common/telemetry/sender';
 import { createDeferred } from '../../../common/utils/deferred';
-import { MANAGER_READY_TIMEOUT_MS, withManagerTimeout } from '../../../features/common/managerReady';
+import * as windowApis from '../../../common/window.apis';
+import {
+    MANAGER_READY_TIMEOUT_MS,
+    _resetManagerReadyForTesting,
+    createManagerReady,
+    waitForEnvManagerId,
+    waitForPkgManagerId,
+    withManagerTimeout,
+} from '../../../features/common/managerReady';
+import * as settingHelpers from '../../../features/settings/settingHelpers';
+import {
+    DidChangeEnvironmentManagerEventArgs,
+    DidChangePackageManagerEventArgs,
+    EnvironmentManagers,
+    InternalEnvironmentManager,
+    InternalPackageManager,
+    PythonProjectManager,
+} from '../../../internal.api';
 
 suite('withManagerTimeout', () => {
     let clock: sinon.SinonFakeTimers;
@@ -104,3 +123,149 @@ suite('withManagerTimeout', () => {
         assert.strictEqual(properties.managerKind, 'package');
     });
 });
+
+suite('ManagerReady - case-insensitive ID matching', () => {
+    let envManagerEmitter: EventEmitter<DidChangeEnvironmentManagerEventArgs>;
+    let pkgManagerEmitter: EventEmitter<DidChangePackageManagerEventArgs>;
+    let clock: sinon.SinonFakeTimers;
+    let disposables: Disposable[];
+
+    setup(() => {
+        clock = sinon.useFakeTimers();
+        disposables = [];
+
+        _resetManagerReadyForTesting();
+
+        envManagerEmitter = new EventEmitter<DidChangeEnvironmentManagerEventArgs>();
+        pkgManagerEmitter = new EventEmitter<DidChangePackageManagerEventArgs>();
+
+        // Stub logging and telemetry to keep test output clean
+        sinon.stub(logging, 'traceWarn');
+        sinon.stub(logging, 'traceError');
+        sinon.stub(logging, 'traceInfo');
+        sinon.stub(telemetrySender, 'sendTelemetryEvent');
+        sinon.stub(windowApis, 'showErrorMessage');
+        sinon.stub(extensionApis, 'getExtension').returns({
+            id: 'ms-python.python',
+            isActive: true,
+        } as unknown as ReturnType<typeof extensionApis.getExtension>);
+        sinon.stub(settingHelpers, 'getDefaultEnvManagerSetting').returns('ms-python.python:venv');
+        sinon.stub(settingHelpers, 'getDefaultPkgManagerSetting').returns('ms-python.python:pip');
+
+        const mockEm = {
+            onDidChangeEnvironmentManager: envManagerEmitter.event,
+            onDidChangePackageManager: pkgManagerEmitter.event,
+        } as unknown as EnvironmentManagers;
+
+        const mockPm = {
+            getProjects: () => [],
+        } as unknown as PythonProjectManager;
+
+        createManagerReady(mockEm, mockPm, disposables);
+    });
+
+    teardown(() => {
+        clock.restore();
+        disposables.forEach((d) => d.dispose());
+        envManagerEmitter.dispose();
+        pkgManagerEmitter.dispose();
+        sinon.restore();
+        _resetManagerReadyForTesting();
+    });
+
+    test('env manager registered with different casing resolves the wait', async () => {
+        // Settings say "ms-python.python:venv" (lowercase), but the registered
+        // manager might have a different-cased extension ID
+        const waitPromise = waitForEnvManagerId(['ms-python.python:venv']);
+
+        // Simulate manager registration with mixed-case ID
+        envManagerEmitter.fire({
+            kind: 'registered',
+            manager: { id: 'Ms-Python.Python:venv' } as unknown as InternalEnvironmentManager,
+        });
+
+        await clock.tickAsync(0);
+        await waitPromise;
+
+        // If we get here, the deferred resolved — no timeout needed
+    });
+
+    test('pkg manager registered with different casing resolves the wait', async () => {
+        const waitPromise = waitForPkgManagerId(['ms-python.python:pip']);
+
+        pkgManagerEmitter.fire({
+            kind: 'registered',
+            manager: { id: 'Ms-Python.Python:pip' } as unknown as InternalPackageManager,
+        });
+
+        await clock.tickAsync(0);
+        await waitPromise;
+    });
+
+    test('manager registered before wait is called still resolves (case-insensitive)', async () => {
+        // Manager registers first with mixed case
+        envManagerEmitter.fire({
+            kind: 'registered',
+            manager: { id: 'MS-PYTHON.PYTHON:venv' } as unknown as InternalEnvironmentManager,
+        });
+
+        await clock.tickAsync(0);
+
+        // Then we wait for the lowercase version from settings — should resolve immediately
+        const waitPromise = waitForEnvManagerId(['ms-python.python:venv']);
+        await clock.tickAsync(0);
+        await waitPromise;
+    });
+
+    test('checkExtension uses getExtension (case-insensitive) not allExtensions scan', async () => {
+        const getExtensionStub = extensionApis.getExtension as sinon.SinonStub;
+        getExtensionStub.withArgs('ms-python.python').returns({
+            id: 'ms-python.python',
+            isActive: true,
+        });
+
+        // Trigger checkExtension via waitForEnvManagerId
+        const waitPromise = waitForEnvManagerId(['ms-python.python:venv']);
+
+        // Resolve the manager to avoid timeout
+        envManagerEmitter.fire({
+            kind: 'registered',
+            manager: { id: 'ms-python.python:venv' } as unknown as InternalEnvironmentManager,
+        });
+
+        await clock.tickAsync(0);
+        await waitPromise;
+
+        // getExtension should have been called (it's case-insensitive in VS Code)
+        assert.ok(getExtensionStub.called, 'getExtension should be used for extension lookup');
+
+        // showErrorMessage should NOT have been called since the extension is found
+        const showErrorStub = windowApis.showErrorMessage as sinon.SinonStub;
+        assert.ok(!showErrorStub.called, 'should not prompt to install when extension is found via getExtension');
+    });
+
+    test('checkExtension deduplicates mixed-case manager IDs', async () => {
+        const getExtensionStub = extensionApis.getExtension as sinon.SinonStub;
+        getExtensionStub.returns({ id: 'ms-python.python', isActive: true });
+
+        // Wait for the same manager ID in different casings
+        const wait1 = waitForEnvManagerId(['ms-python.python:venv']);
+        const wait2 = waitForEnvManagerId(['Ms-Python.Python:venv']);
+
+        envManagerEmitter.fire({
+            kind: 'registered',
+            manager: { id: 'ms-python.python:venv' } as unknown as InternalEnvironmentManager,
+        });
+
+        await clock.tickAsync(0);
+        await Promise.all([wait1, wait2]);
+
+        // Run any pending setImmediate callbacks
+        clock.tick(0);
+        await clock.tickAsync(0);
+
+        // getExtension should only be called once (deduplicated by normalized key)
+        const callsForPython = getExtensionStub.getCalls().filter((c: sinon.SinonSpyCall) => c.args[0] === 'ms-python.python');
+        assert.strictEqual(callsForPython.length, 1, 'checkExtension should deduplicate case-insensitive manager IDs');
+    });
+});
