fix .env file issue

Author: StellaHuang95

src/features/terminal/terminalEnvVarInjector.ts

@@ -149,6 +149,8 @@ export class TerminalEnvVarInjector implements Disposable {
                 traceVerbose(
                     `TerminalEnvVarInjector: Env file injection disabled for workspace: ${workspaceUri.fsPath}`,
                 );
+                // Clear any previously set variables when injection is disabled
+                envVarScope.clear();
                 return;
             }
 
@@ -165,14 +167,18 @@ export class TerminalEnvVarInjector implements Disposable {
                 traceVerbose(
                     `TerminalEnvVarInjector: No .env file found for workspace: ${workspaceUri.fsPath}, not injecting environment variables.`,
                 );
-                return; // No .env file to inject
+                // Clear any previously set variables when no .env file exists
+                envVarScope.clear();
+                return;
             }
 
+            // Clear all previously set variables before re-injecting.
+            // This ensures that when variables are commented out or removed from .env,
+            // they are properly removed from the terminal environment.
+            envVarScope.clear();
+
             for (const [key, value] of Object.entries(envVars)) {
-                if (value === undefined) {
-                    // Remove the environment variable if the value is undefined
-                    envVarScope.delete(key);
-                } else {
+                if (value !== undefined) {
                     envVarScope.replace(key, value);
                 }
             }

src/test/features/terminalEnvVarInjectorBasic.unit.test.ts

@@ -3,7 +3,8 @@
 
 import * as sinon from 'sinon';
 import * as typeMoq from 'typemoq';
-import { GlobalEnvironmentVariableCollection, workspace } from 'vscode';
+import { GlobalEnvironmentVariableCollection, Uri, workspace, WorkspaceFolder } from 'vscode';
+import * as workspaceApis from '../../common/workspace.apis';
 import { EnvVarManager } from '../../features/execution/envVariableManager';
 import { TerminalEnvVarInjector } from '../../features/terminal/terminalEnvVarInjector';
 
@@ -55,8 +56,14 @@ suite('TerminalEnvVarInjector Basic Tests', () => {
                     ({
                         dispose: () => {},
                         // eslint-disable-next-line @typescript-eslint/no-explicit-any
-                    } as any),
+                    }) as any,
             );
+        // Mock workspace.onDidChangeConfiguration to return a proper disposable
+        Object.defineProperty(workspace, 'onDidChangeConfiguration', {
+            value: () => ({ dispose: () => {} }),
+            configurable: true,
+            writable: true,
+        });
     });
 
     teardown(() => {
@@ -102,3 +109,132 @@ suite('TerminalEnvVarInjector Basic Tests', () => {
         sinon.assert.match(eventHandlerRegistered, true);
     });
 });
+
+/**
+ * Tests for variable clearing: Ensure that when .env file variables are commented out or removed,
+ * they are properly removed from the terminal environment.
+ *
+ * These tests verify the clear() behavior when useEnvFile setting is disabled.
+ * Tests for file-existence scenarios are integration-level and not covered here.
+ */
+suite('TerminalEnvVarInjector - Variable Clearing', () => {
+    let envVarCollection: typeMoq.IMock<GlobalEnvironmentVariableCollection>;
+    let injector: TerminalEnvVarInjector;
+    let mockScopedCollection: MockScopedCollection;
+    let mockGetConfiguration: sinon.SinonStub;
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    let workspaceFoldersStub: any;
+    let mockWorkspaceFolder: WorkspaceFolder;
+    let mockEnvVarManager: {
+        onDidChangeEnvironmentVariables: sinon.SinonStub;
+        getEnvironmentVariables: sinon.SinonStub;
+    };
+
+    interface MockWorkspaceConfig {
+        get: sinon.SinonStub;
+    }
+
+    setup(() => {
+        envVarCollection = typeMoq.Mock.ofType<GlobalEnvironmentVariableCollection>();
+
+        // Create mock EnvVarManager using sinon stubs
+        mockEnvVarManager = {
+            onDidChangeEnvironmentVariables: sinon.stub().returns({ dispose: () => {} }),
+            getEnvironmentVariables: sinon.stub().resolves({}),
+        };
+
+        // Create a mock workspace folder
+        mockWorkspaceFolder = {
+            uri: Uri.file('/test/workspace'),
+            name: 'test-workspace',
+            index: 0,
+        };
+
+        // Mock workspace.workspaceFolders property
+        workspaceFoldersStub = [mockWorkspaceFolder];
+        Object.defineProperty(workspace, 'workspaceFolders', {
+            get: () => workspaceFoldersStub,
+            configurable: true,
+        });
+
+        // Setup scoped collection mock
+        mockScopedCollection = {
+            clear: sinon.stub(),
+            replace: sinon.stub(),
+            delete: sinon.stub(),
+        };
+
+        // Setup environment variable collection to return scoped collection
+        envVarCollection
+            .setup((x) => x.getScoped(typeMoq.It.isAny()))
+            .returns(
+                () => mockScopedCollection as unknown as ReturnType<GlobalEnvironmentVariableCollection['getScoped']>,
+            );
+        envVarCollection.setup((x) => x.clear()).returns(() => {});
+
+        // Mock getConfiguration
+        mockGetConfiguration = sinon.stub(workspaceApis, 'getConfiguration');
+
+        // Mock workspace.onDidChangeConfiguration to return a proper disposable
+        Object.defineProperty(workspace, 'onDidChangeConfiguration', {
+            value: () => ({ dispose: () => {} }),
+            configurable: true,
+            writable: true,
+        });
+    });
+
+    teardown(() => {
+        sinon.restore();
+        injector?.dispose();
+    });
+
+    test('should call clear() when useEnvFile setting is disabled', async () => {
+        // Arrange - mock configuration with useEnvFile disabled
+        const mockConfig: MockWorkspaceConfig = {
+            get: sinon.stub(),
+        };
+        mockConfig.get.withArgs('terminal.useEnvFile', false).returns(false);
+        mockConfig.get.withArgs('envFile').returns(undefined);
+        mockGetConfiguration.returns(mockConfig);
+
+        // Mock getEnvironmentVariables
+        mockEnvVarManager.getEnvironmentVariables.resolves({ TEST_VAR: 'value' });
+
+        // Act
+        injector = new TerminalEnvVarInjector(envVarCollection.object, mockEnvVarManager as unknown as EnvVarManager);
+
+        // Wait for async initialization
+        await new Promise((resolve) => setTimeout(resolve, 150));
+
+        // Assert - clear() should be called, but replace() should NOT be called
+        sinon.assert.called(mockScopedCollection.clear);
+        sinon.assert.notCalled(mockScopedCollection.replace);
+    });
+
+    test('should not inject variables when useEnvFile is disabled even if env vars exist', async () => {
+        // Arrange - mock configuration with useEnvFile disabled
+        const mockConfig: MockWorkspaceConfig = {
+            get: sinon.stub(),
+        };
+        mockConfig.get.withArgs('terminal.useEnvFile', false).returns(false);
+        mockConfig.get.withArgs('envFile').returns('.env');
+        mockGetConfiguration.returns(mockConfig);
+
+        // Mock getEnvironmentVariables to return multiple variables
+        mockEnvVarManager.getEnvironmentVariables.resolves({
+            API_KEY: 'secret123',
+            DATABASE_URL: 'postgres://localhost/db',
+            DEBUG: 'true',
+        });
+
+        // Act
+        injector = new TerminalEnvVarInjector(envVarCollection.object, mockEnvVarManager as unknown as EnvVarManager);
+
+        // Wait for async initialization
+        await new Promise((resolve) => setTimeout(resolve, 150));
+
+        // Assert - clear() should be called to remove any previous variables, but replace() should NOT be called
+        sinon.assert.called(mockScopedCollection.clear);
+        sinon.assert.notCalled(mockScopedCollection.replace);
+    });
+});