Fix: Do not activate on pseudoterminals (#1509)

edvilme · web-flow · commit 8fc6694caafa · 2026-05-20T10:39:19.000-07:00
This pull request introduces a new utility function to consistently determine when terminal activation should be skipped, and refactors related logic throughout the codebase to use this function. The main goal is to improve reliability and maintainability by centralizing the logic for skipping activation of certain terminals (such as hidden or PTY-based extension terminals). **Terminal activation skip logic improvements:** * Added a new function `shouldSkipTerminalActivation` in `src/features/terminal/utils.ts` to centralize the logic for skipping activation for terminals that are hidden from the user or are PTY-based extension terminals. * Refactored `TerminalManagerImpl` in `src/features/terminal/terminalManager.ts` to use `shouldSkipTerminalActivation` instead of directly checking terminal options in multiple places, ensuring consistent behavior when opening or activating terminals. [[1]](diffhunk://#diff-1abe443a8980fdf92f537c69abb866fdb65ca6a00be94942edc99eca49d60d75R37) [[2]](diffhunk://#diff-1abe443a8980fdf92f537c69abb866fdb65ca6a00be94942edc99eca49d60d75L97-R98) [[3]](diffhunk://#diff-1abe443a8980fdf92f537c69abb866fdb65ca6a00be94942edc99eca49d60d75L422-R427) [[4]](diffhunk://#diff-1abe443a8980fdf92f537c69abb866fdb65ca6a00be94942edc99eca49d60d75L434-R441) * Updated `TerminalActivationImpl` in `src/features/terminal/terminalActivationState.ts` to use the new skip logic, preventing activation for terminals that meet the skip criteria. [[1]](diffhunk://#diff-4c638af1ef4f02d8c8038fcc38ca4a135ca30aaa3f2f51d840a2f5f372ffabbdL14-R14) [[2]](diffhunk://#diff-4c638af1ef4f02d8c8038fcc38ca4a135ca30aaa3f2f51d840a2f5f372ffabbdR89-R93) **Other changes:** * Cleaned up imports in `src/features/terminal/terminalManager.ts` and `src/features/terminal/utils.ts` to remove unused `TerminalOptions` and add `ExtensionTerminalOptions` where needed. [[1]](diffhunk://#diff-1abe443a8980fdf92f537c69abb866fdb65ca6a00be94942edc99eca49d60d75L3-R3) [[2]](diffhunk://#diff-da71e7d6ace45ba8e09cc2f84096dcd5149997f178e5c0dafd84bbcd4627c822L2-R2) Fixes #1482
diff --git a/src/features/terminal/terminalActivationState.ts b/src/features/terminal/terminalActivationState.ts
@@ -11,7 +11,7 @@ import { PythonEnvironment } from '../../api';
 import { traceError, traceInfo, traceVerbose } from '../../common/logging';
 import { onDidEndTerminalShellExecution, onDidStartTerminalShellExecution } from '../../common/window.apis';
 import { getActivationCommand, getDeactivationCommand } from '../common/activation';
-import { getShellIntegrationTimeout, isTaskTerminal } from './utils';
+import { getShellIntegrationTimeout, isTaskTerminal, shouldSkipTerminalActivation } from './utils';
 
 export interface DidChangeTerminalActivationStateEvent {
     terminal: Terminal;
@@ -86,6 +86,11 @@ export class TerminalActivationImpl implements TerminalActivationInternal {
     }
 
     async activate(terminal: Terminal, environment: PythonEnvironment): Promise<void> {
+        if (shouldSkipTerminalActivation(terminal)) {
+            traceVerbose('Skipping activation for this terminal');
+            return;
+        }
+
         if (isTaskTerminal(terminal)) {
             traceVerbose('Cannot activate environment in a task terminal');
             return;
diff --git a/src/features/terminal/terminalManager.ts b/src/features/terminal/terminalManager.ts
@@ -1,6 +1,6 @@
 import * as fsapi from 'fs-extra';
 import * as path from 'path';
-import { Disposable, EventEmitter, ProgressLocation, Terminal, TerminalOptions, Uri } from 'vscode';
+import { Disposable, EventEmitter, ProgressLocation, Terminal, Uri } from 'vscode';
 import { PythonEnvironment, PythonEnvironmentApi, PythonProject, PythonTerminalCreateOptions } from '../../api';
 import { ActivationStrings } from '../../common/localize';
 import { traceInfo, traceVerbose } from '../../common/logging';
@@ -34,6 +34,7 @@ import {
     getAutoActivationType,
     getEnvironmentForTerminal,
     shouldActivateInCurrentTerminal,
+    shouldSkipTerminalActivation,
     waitForShellIntegration,
 } from './utils';
 
@@ -94,7 +95,7 @@ export class TerminalManagerImpl implements TerminalManager {
                 this.onTerminalClosedEmitter.fire(t);
             }),
             this.onTerminalOpened(async (t) => {
-                if (this.skipActivationOnOpen.has(t) || (t.creationOptions as TerminalOptions)?.hideFromUser) {
+                if (this.skipActivationOnOpen.has(t) || shouldSkipTerminalActivation(t)) {
                     return;
                 }
                 let env = this.ta.getEnvironment(t);
@@ -419,7 +420,11 @@ export class TerminalManagerImpl implements TerminalManager {
 
         if (actType === ACT_TYPE_COMMAND) {
             if (!skipPreExistingTerminals) {
-                await Promise.all(terminals().map(async (t) => this.activateUsingCommand(api, t)));
+                await Promise.all(
+                    terminals()
+                        .filter((t) => !shouldSkipTerminalActivation(t))
+                        .map(async (t) => this.activateUsingCommand(api, t)),
+                );
             }
         } else if (actType === ACT_TYPE_SHELL) {
             const shells = new Set(
@@ -431,12 +436,14 @@ export class TerminalManagerImpl implements TerminalManager {
                 await this.handleSetupCheck(shells);
                 if (!skipPreExistingTerminals) {
                     await Promise.all(
-                        terminals().map(async (t) => {
-                            // If the shell is not set up, we activate using command fallback.
-                            if (this.shellSetup.get(identifyTerminalShell(t)) === false) {
-                                await this.activateUsingCommand(api, t);
-                            }
-                        }),
+                        terminals()
+                            .filter((t) => !shouldSkipTerminalActivation(t))
+                            .map(async (t) => {
+                                // If the shell is not set up, we activate using command fallback.
+                                if (this.shellSetup.get(identifyTerminalShell(t)) === false) {
+                                    await this.activateUsingCommand(api, t);
+                                }
+                            }),
                     );
                 }
             }
diff --git a/src/features/terminal/utils.ts b/src/features/terminal/utils.ts
@@ -1,5 +1,5 @@
 import * as path from 'path';
-import { Disposable, env, tasks, Terminal, TerminalOptions, Uri } from 'vscode';
+import { Disposable, env, ExtensionTerminalOptions, tasks, Terminal, TerminalOptions, Uri } from 'vscode';
 import { PythonEnvironment, PythonProject, PythonProjectEnvironmentApi, PythonProjectGetterApi } from '../../api';
 import { traceVerbose } from '../../common/logging';
 import { timeout } from '../../common/utils/asyncUtils';
@@ -385,3 +385,16 @@ export function removeAnsiEscapeCodes(str: string): string {
 
     return str;
 }
+
+/**
+ * Determines if a terminal should be skipped for environment activation based on its creation options.
+ * Terminals that are hidden from the user or are PTY-based extension terminals are skipped.
+ * @param terminal The terminal to check.
+ * @returns `true` if the terminal should be skipped; `false` otherwise.
+ */
+export function shouldSkipTerminalActivation(terminal: Terminal): boolean {
+    return (
+        !!(terminal.creationOptions as TerminalOptions)?.hideFromUser ||
+        !!(terminal.creationOptions as ExtensionTerminalOptions)?.pty
+    );
+}
diff --git a/src/test/features/terminal/terminalManager.unit.test.ts b/src/test/features/terminal/terminalManager.unit.test.ts
@@ -10,7 +10,9 @@ import {
     Disposable,
     Event,
     EventEmitter,
+    ExtensionTerminalOptions,
     Progress,
+    Pseudoterminal,
     Terminal,
     TerminalOptions,
     Uri,
@@ -617,3 +619,147 @@ suite('TerminalManager - initialize() with activateEnvInCurrentTerminal', () =>
         );
     });
 });
+
+suite('TerminalManager - initialize() skips pseudoterminals', () => {
+    let terminalActivation: TestTerminalActivation;
+    let terminalManager: TerminalManagerImpl;
+    let mockGetAutoActivationType: sinon.SinonStub;
+    let mockShouldActivateInCurrentTerminal: sinon.SinonStub;
+    let mockTerminals: sinon.SinonStub;
+    let mockGetEnvironmentForTerminal: sinon.SinonStub;
+
+    const createMockPty = (): Pseudoterminal => {
+        const writeEmitter = new EventEmitter<string>();
+        return { onDidWrite: writeEmitter.event, open: () => {}, close: () => {} };
+    };
+
+    const createMockExtensionTerminalOptions = (name: string): ExtensionTerminalOptions => ({
+        name,
+        pty: createMockPty(),
+    });
+
+    const createMockTerminal = (name: string, options?: TerminalOptions | ExtensionTerminalOptions): Terminal =>
+        ({
+            name,
+            creationOptions: options ?? ({} as TerminalOptions),
+            shellIntegration: undefined,
+            show: sinon.stub(),
+            sendText: sinon.stub(),
+        }) as unknown as Terminal;
+
+    const createMockEnvironment = (): PythonEnvironment => ({
+        envId: { id: 'test-env-id', managerId: 'test-manager' },
+        name: 'Test Environment',
+        displayName: 'Test Environment',
+        shortDisplayName: 'TestEnv',
+        displayPath: '/path/to/env',
+        version: '3.9.0',
+        environmentPath: Uri.file('/path/to/python'),
+        sysPrefix: '/path/to/env',
+        execInfo: {
+            run: { executable: '/path/to/python' },
+            activation: [{ executable: '/path/to/activate' }],
+        },
+    });
+
+    setup(() => {
+        terminalActivation = new TestTerminalActivation();
+
+        mockGetAutoActivationType = sinon.stub(terminalUtils, 'getAutoActivationType');
+        mockShouldActivateInCurrentTerminal = sinon.stub(terminalUtils, 'shouldActivateInCurrentTerminal');
+        mockGetEnvironmentForTerminal = sinon.stub(terminalUtils, 'getEnvironmentForTerminal');
+        sinon.stub(terminalUtils, 'waitForShellIntegration').resolves(false);
+        sinon.stub(activationUtils, 'isActivatableEnvironment').returns(true);
+        sinon.stub(shellDetector, 'identifyTerminalShell').returns('bash');
+
+        sinon.stub(windowApis, 'createTerminal').callsFake(() => createMockTerminal('new'));
+        sinon.stub(windowApis, 'onDidOpenTerminal').returns(new Disposable(() => {}));
+        sinon.stub(windowApis, 'onDidCloseTerminal').returns(new Disposable(() => {}));
+        sinon.stub(windowApis, 'onDidChangeWindowState').returns(new Disposable(() => {}));
+        sinon.stub(windowApis, 'activeTerminal').returns(undefined);
+
+        mockTerminals = sinon.stub(windowApis, 'terminals');
+
+        sinon.stub(windowApis, 'withProgress').callsFake(async (_options, task) => {
+            const mockProgress = { report: () => {} };
+            const mockToken = {
+                isCancellationRequested: false,
+                onCancellationRequested: () => new Disposable(() => {}),
+            };
+            return task(mockProgress as never, mockToken as never);
+        });
+        sinon.stub(workspaceApis, 'onDidChangeConfiguration').returns(new Disposable(() => {}));
+    });
+
+    teardown(() => {
+        sinon.restore();
+        terminalActivation.dispose();
+    });
+
+    test('initialize skips pseudoterminals and activates regular terminals (ACT_TYPE_COMMAND)', async () => {
+        const regularTerminal = createMockTerminal('regular');
+        const pseudoTerminal = createMockTerminal('pseudo', createMockExtensionTerminalOptions('pseudo'));
+        const env = createMockEnvironment();
+
+        mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_COMMAND);
+        mockShouldActivateInCurrentTerminal.returns(true);
+        mockTerminals.returns([regularTerminal, pseudoTerminal]);
+        mockGetEnvironmentForTerminal.resolves(env);
+
+        terminalManager = new TerminalManagerImpl(terminalActivation, [], []);
+        await terminalManager.initialize({} as never);
+
+        assert.strictEqual(
+            terminalActivation.activateCalls,
+            1,
+            'Should only activate the regular terminal, not the pseudoterminal',
+        );
+    });
+
+    test('initialize skips pseudoterminals in shell startup fallback path (ACT_TYPE_SHELL)', async () => {
+        const regularTerminal = createMockTerminal('regular');
+        const pseudoTerminal = createMockTerminal('pseudo', createMockExtensionTerminalOptions('pseudo'));
+        const env = createMockEnvironment();
+
+        const mockShellProvider: ShellStartupScriptProvider = {
+            name: 'bash-test',
+            shellType: 'bash',
+            isSetup: sinon.stub().resolves(ShellSetupState.NotSetup),
+            setupScripts: sinon.stub().resolves(ShellScriptEditState.NotEdited),
+            teardownScripts: sinon.stub().resolves(ShellScriptEditState.NotEdited),
+            clearCache: sinon.stub().resolves(),
+        };
+        sinon.stub(shellUtils, 'getShellIntegrationEnabledCache').resolves(false);
+        sinon.stub(shellUtils, 'shouldUseProfileActivation').returns(false);
+
+        mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_SHELL);
+        mockShouldActivateInCurrentTerminal.returns(true);
+        mockTerminals.returns([regularTerminal, pseudoTerminal]);
+        mockGetEnvironmentForTerminal.resolves(env);
+
+        terminalManager = new TerminalManagerImpl(terminalActivation, [], [mockShellProvider]);
+        await terminalManager.initialize({} as never);
+
+        assert.strictEqual(
+            terminalActivation.activateCalls,
+            1,
+            'Should only activate the regular terminal via command fallback, not the pseudoterminal',
+        );
+    });
+
+    test('initialize skips all terminals when only pseudoterminals exist (ACT_TYPE_COMMAND)', async () => {
+        const pseudo1 = createMockTerminal('pseudo1', createMockExtensionTerminalOptions('pseudo1'));
+        const pseudo2 = createMockTerminal('pseudo2', createMockExtensionTerminalOptions('pseudo2'));
+        const env = createMockEnvironment();
+
+        mockGetAutoActivationType.returns(terminalUtils.ACT_TYPE_COMMAND);
+        mockShouldActivateInCurrentTerminal.returns(true);
+        mockTerminals.returns([pseudo1, pseudo2]);
+        mockGetEnvironmentForTerminal.resolves(env);
+
+        terminalManager = new TerminalManagerImpl(terminalActivation, [], []);
+        await terminalManager.initialize({} as never);
+
+        assert.strictEqual(terminalActivation.activateCalls, 0, 'Should not activate any pseudoterminals');
+    });
+});
diff --git a/src/test/features/terminal/utils.unit.test.ts b/src/test/features/terminal/utils.unit.test.ts
@@ -1,6 +1,6 @@
 import * as assert from 'assert';
 import * as sinon from 'sinon';
-import { Terminal } from 'vscode';
+import { ExtensionTerminalOptions, Terminal, TerminalOptions } from 'vscode';
 import * as windowApis from '../../../common/window.apis';
 import * as workspaceApis from '../../../common/workspace.apis';
 import * as shellDetector from '../../../features/common/shellDetector';
@@ -11,6 +11,7 @@ import {
     AutoActivationType,
     getAutoActivationType,
     shouldActivateInCurrentTerminal,
+    shouldSkipTerminalActivation,
     waitForShellIntegration,
 } from '../../../features/terminal/utils';
 
@@ -550,6 +551,45 @@ suite('Terminal Utils - shouldActivateInCurrentTerminal', () => {
     });
 });
 
+suite('Terminal Utils - shouldSkipTerminalActivation', () => {
+    test('should return false for a regular terminal with no special options', () => {
+        const terminal = { creationOptions: {} as TerminalOptions } as Terminal;
+        assert.strictEqual(shouldSkipTerminalActivation(terminal), false);
+    });
+
+    test('should return true when hideFromUser is true', () => {
+        const terminal = { creationOptions: { hideFromUser: true } as TerminalOptions } as Terminal;
+        assert.strictEqual(shouldSkipTerminalActivation(terminal), true);
+    });
+
+    test('should return false when hideFromUser is false', () => {
+        const terminal = { creationOptions: { hideFromUser: false } as TerminalOptions } as Terminal;
+        assert.strictEqual(shouldSkipTerminalActivation(terminal), false);
+    });
+
+    test('should return true for a pseudoterminal (pty-based extension terminal)', () => {
+        const terminal = {
+            creationOptions: {
+                name: 'pseudo',
+                pty: { open: () => {}, close: () => {} },
+            } as unknown as ExtensionTerminalOptions,
+        } as Terminal;
+        assert.strictEqual(shouldSkipTerminalActivation(terminal), true);
+    });
+
+    test('should return false when pty is undefined', () => {
+        const terminal = { creationOptions: {} as ExtensionTerminalOptions } as Terminal;
+        assert.strictEqual(shouldSkipTerminalActivation(terminal), false);
+    });
+
+    test('should return true when both hideFromUser and pty are set', () => {
+        const terminal = {
+            creationOptions: { hideFromUser: true, pty: { open: () => {}, close: () => {} } },
+        } as unknown as Terminal;
+        assert.strictEqual(shouldSkipTerminalActivation(terminal), true);
+    });
+});
+
 suite('Terminal Utils - waitForShellIntegration', () => {
     let mockGetConfiguration: sinon.SinonStub;
     let identifyTerminalShellStub: sinon.SinonStub;
