Prevent double/triple activation from two extensions

Author: anthonykim1

src/client/common/terminal/activator/index.ts

@@ -9,7 +9,7 @@ import { IConfigurationService, IExperimentService } from '../../types';
 import { ITerminalActivationHandler, ITerminalActivator, ITerminalHelper, TerminalActivationOptions } from '../types';
 import { BaseTerminalActivator } from './base';
 import { inTerminalEnvVarExperiment } from '../../experiments/helpers';
-import { useEnvExtension } from '../../../envExt/api.internal';
+import { shouldEnvExtHandleActivation } from '../../../envExt/api.internal';
 import { EventName } from '../../../telemetry/constants';
 import { sendTelemetryEvent } from '../../../telemetry';
 
@@ -44,8 +44,8 @@ export class TerminalActivator implements ITerminalActivator {
         const settings = this.configurationService.getSettings(options?.resource);
         const activateEnvironment =
             settings.terminal.activateEnvironment && !inTerminalEnvVarExperiment(this.experimentService);
-        if (!activateEnvironment || options?.hideFromUser || useEnvExtension()) {
-            if (useEnvExtension()) {
+        if (!activateEnvironment || options?.hideFromUser || shouldEnvExtHandleActivation()) {
+            if (shouldEnvExtHandleActivation()) {
                 sendTelemetryEvent(EventName.PYTHON_INTERPRETER_ACTIVATION_FOR_TERMINAL);
             }
             return false;

src/client/envExt/api.internal.ts

@@ -19,6 +19,27 @@ import { Interpreters } from '../common/utils/localize';
 
 export const ENVS_EXTENSION_ID = 'ms-python.vscode-python-envs';
 
+export function isEnvExtensionInstalled(): boolean {
+    return !!getExtension(ENVS_EXTENSION_ID);
+}
+
+/**
+ * Returns true if the Python Environments extension is installed and not explicitly
+ * disabled by the user. Mirrors the envs extension's own activation logic: it
+ * deactivates only when `python.useEnvironmentsExtension` is explicitly set to false.
+ */
+export function shouldEnvExtHandleActivation(): boolean {
+    if (!isEnvExtensionInstalled()) {
+        return false;
+    }
+    const config = getConfiguration('python');
+    const inspection = config.inspect<boolean>('useEnvironmentsExtension');
+    if (inspection?.globalValue === false || inspection?.workspaceValue === false) {
+        return false;
+    }
+    return true;
+}
+
 let _useExt: boolean | undefined;
 export function useEnvExtension(): boolean {
     if (_useExt !== undefined) {

src/client/providers/terminalProvider.ts

@@ -11,7 +11,7 @@ import { swallowExceptions } from '../common/utils/decorators';
 import { IServiceContainer } from '../ioc/types';
 import { captureTelemetry, sendTelemetryEvent } from '../telemetry';
 import { EventName } from '../telemetry/constants';
-import { useEnvExtension } from '../envExt/api.internal';
+import { useEnvExtension, shouldEnvExtHandleActivation } from '../envExt/api.internal';
 
 export class TerminalProvider implements Disposable {
     private disposables: Disposable[] = [];
@@ -33,7 +33,7 @@ export class TerminalProvider implements Disposable {
             currentTerminal &&
             pythonSettings.terminal.activateEnvInCurrentTerminal &&
             !inTerminalEnvVarExperiment(experimentService) &&
-            !useEnvExtension()
+            !shouldEnvExtHandleActivation()
         ) {
             const hideFromUser =
                 'hideFromUser' in currentTerminal.creationOptions && currentTerminal.creationOptions.hideFromUser;

src/client/terminals/activation.ts

@@ -9,6 +9,7 @@ import { IActiveResourceService, ITerminalManager } from '../common/application/
 import { ITerminalActivator } from '../common/terminal/types';
 import { IDisposable, IDisposableRegistry } from '../common/types';
 import { ITerminalAutoActivation } from './types';
+import { shouldEnvExtHandleActivation } from '../envExt/api.internal';
 
 @injectable()
 export class TerminalAutoActivation implements ITerminalAutoActivation {
@@ -49,6 +50,9 @@ export class TerminalAutoActivation implements ITerminalAutoActivation {
         if (this.terminalsNotToAutoActivate.has(terminal)) {
             return;
         }
+        if (shouldEnvExtHandleActivation()) {
+            return;
+        }
         if ('hideFromUser' in terminal.creationOptions && terminal.creationOptions.hideFromUser) {
             return;
         }

src/test/common/terminals/activator/index.unit.test.ts

@@ -20,6 +20,8 @@ import {
     ITerminalSettings,
 } from '../../../../client/common/types';
 import * as extapi from '../../../../client/envExt/api.internal';
+import * as workspaceApis from '../../../../client/common/vscodeApis/workspaceApis';
+import * as extensionsApi from '../../../../client/common/vscodeApis/extensionsApi';
 
 suite('Terminal Activator', () => {
     let activator: TerminalActivator;
@@ -29,9 +31,12 @@ suite('Terminal Activator', () => {
     let terminalSettings: TypeMoq.IMock<ITerminalSettings>;
     let experimentService: TypeMoq.IMock<IExperimentService>;
     let useEnvExtensionStub: sinon.SinonStub;
+    let shouldEnvExtHandleActivationStub: sinon.SinonStub;
     setup(() => {
         useEnvExtensionStub = sinon.stub(extapi, 'useEnvExtension');
         useEnvExtensionStub.returns(false);
+        shouldEnvExtHandleActivationStub = sinon.stub(extapi, 'shouldEnvExtHandleActivation');
+        shouldEnvExtHandleActivationStub.returns(false);
 
         baseActivator = TypeMoq.Mock.ofType<ITerminalActivator>();
         terminalSettings = TypeMoq.Mock.ofType<ITerminalSettings>();
@@ -113,4 +118,72 @@ suite('Terminal Activator', () => {
     test('Terminal is not activated if auto-activate setting is set to true but terminal is hidden', () =>
         testActivationAndHandlers(false, true, true));
     test('Terminal is not activated and handlers are invoked', () => testActivationAndHandlers(false, false));
+
+    test('Terminal is not activated from Python extension when Env extension should handle activation', async () => {
+        shouldEnvExtHandleActivationStub.returns(true);
+        terminalSettings.setup((b) => b.activateEnvironment).returns(() => true);
+        baseActivator
+            .setup((b) => b.activateEnvironmentInTerminal(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
+            .returns(() => Promise.resolve(true))
+            .verifiable(TypeMoq.Times.never());
+
+        const terminal = TypeMoq.Mock.ofType<Terminal>();
+        const activated = await activator.activateEnvironmentInTerminal(terminal.object, {
+            preserveFocus: true,
+        });
+
+        assert.strictEqual(activated, false);
+        baseActivator.verifyAll();
+    });
+});
+
+suite('shouldEnvExtHandleActivation', () => {
+    let getExtensionStub: sinon.SinonStub;
+    let getConfigurationStub: sinon.SinonStub;
+
+    setup(() => {
+        getExtensionStub = sinon.stub(extensionsApi, 'getExtension');
+        getConfigurationStub = sinon.stub(workspaceApis, 'getConfiguration');
+    });
+
+    teardown(() => {
+        sinon.restore();
+    });
+
+    test('Returns false when envs extension is not installed', () => {
+        getExtensionStub.returns(undefined);
+        assert.strictEqual(extapi.shouldEnvExtHandleActivation(), false);
+    });
+
+    test('Returns true when envs extension is installed and setting is not explicitly set', () => {
+        getExtensionStub.returns({ id: extapi.ENVS_EXTENSION_ID });
+        getConfigurationStub.returns({
+            inspect: () => ({ globalValue: undefined, workspaceValue: undefined }),
+        });
+        assert.strictEqual(extapi.shouldEnvExtHandleActivation(), true);
+    });
+
+    test('Returns false when envs extension is installed but globalValue is false', () => {
+        getExtensionStub.returns({ id: extapi.ENVS_EXTENSION_ID });
+        getConfigurationStub.returns({
+            inspect: () => ({ globalValue: false, workspaceValue: undefined }),
+        });
+        assert.strictEqual(extapi.shouldEnvExtHandleActivation(), false);
+    });
+
+    test('Returns false when envs extension is installed but workspaceValue is false', () => {
+        getExtensionStub.returns({ id: extapi.ENVS_EXTENSION_ID });
+        getConfigurationStub.returns({
+            inspect: () => ({ globalValue: undefined, workspaceValue: false }),
+        });
+        assert.strictEqual(extapi.shouldEnvExtHandleActivation(), false);
+    });
+
+    test('Returns true when envs extension is installed and setting is explicitly true', () => {
+        getExtensionStub.returns({ id: extapi.ENVS_EXTENSION_ID });
+        getConfigurationStub.returns({
+            inspect: () => ({ globalValue: true, workspaceValue: undefined }),
+        });
+        assert.strictEqual(extapi.shouldEnvExtHandleActivation(), true);
+    });
 });

src/test/providers/terminal.unit.test.ts

@@ -29,10 +29,13 @@ suite('Terminal Provider', () => {
     let experimentService: TypeMoq.IMock<IExperimentService>;
     let terminalProvider: TerminalProvider;
     let useEnvExtensionStub: sinon.SinonStub;
+    let shouldEnvExtHandleActivationStub: sinon.SinonStub;
     const resource = Uri.parse('a');
     setup(() => {
         useEnvExtensionStub = sinon.stub(extapi, 'useEnvExtension');
         useEnvExtensionStub.returns(false);
+        shouldEnvExtHandleActivationStub = sinon.stub(extapi, 'shouldEnvExtHandleActivation');
+        shouldEnvExtHandleActivationStub.returns(false);
 
         serviceContainer = TypeMoq.Mock.ofType<IServiceContainer>();
         commandManager = TypeMoq.Mock.ofType<ICommandManager>();

src/test/terminals/activation.unit.test.ts

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 import { anything, instance, mock, verify, when } from 'ts-mockito';
+import * as sinon from 'sinon';
 import { EventEmitter, Terminal } from 'vscode';
 import { ActiveResourceService } from '../../client/common/application/activeResource';
 import { TerminalManager } from '../../client/common/application/terminalManager';
@@ -11,6 +12,7 @@ import { ITerminalActivator } from '../../client/common/terminal/types';
 import { TerminalAutoActivation } from '../../client/terminals/activation';
 import { ITerminalAutoActivation } from '../../client/terminals/types';
 import { noop } from '../core';
+import * as extapi from '../../client/envExt/api.internal';
 
 suite('Terminal', () => {
     suite('Terminal Auto Activation', () => {
@@ -21,8 +23,12 @@ suite('Terminal', () => {
         let onDidOpenTerminalEventEmitter: EventEmitter<Terminal>;
         let terminal: Terminal;
         let nonActivatedTerminal: Terminal;
+        let shouldEnvExtHandleActivationStub: sinon.SinonStub;
 
         setup(() => {
+            shouldEnvExtHandleActivationStub = sinon.stub(extapi, 'shouldEnvExtHandleActivation');
+            shouldEnvExtHandleActivationStub.returns(false);
+
             manager = mock(TerminalManager);
             activator = mock(TerminalActivator);
             resourceService = mock(ActiveResourceService);
@@ -60,6 +66,9 @@ suite('Terminal', () => {
             autoActivation.register();
         });
         // teardown(() => fakeTimer.uninstall());
+        teardown(() => {
+            sinon.restore();
+        });
 
         test('Should activate terminal', async () => {
             // Trigger opening a terminal.
@@ -77,5 +86,12 @@ suite('Terminal', () => {
             // The terminal should get activated.
             verify(activator.activateEnvironmentInTerminal(anything(), anything())).never();
         });
+        test('Should not activate terminal when envs extension should handle activation', async () => {
+            shouldEnvExtHandleActivationStub.returns(true);
+
+            await ((onDidOpenTerminalEventEmitter.fire(terminal) as unknown) as Promise<void>);
+
+            verify(activator.activateEnvironmentInTerminal(anything(), anything())).never();
+        });
     });
 });