Preserve shell-startup activation when environment files change (#1903)

Creating a project `.env` file could clear the terminal’s scoped
variable collection, removing shell-startup activation even when
environment-file injection was disabled.

- **Event handling:** Refresh environment-file variables on create,
edit, and delete events instead of clearing the entire workspace scope.
- **File selection:** Continue using the project `.env` when a
configured environment file is removed, preserving variables from files
that remain.
- **Regression coverage:** Cover injection enabled and disabled,
variable updates and removal, and deletion when another environment file
remains.

- **Concurrency:** Ignore stale asynchronous refreshes when a newer
environment-file event has already completed. Tests now synchronize
directly with refresh completion and cover out-of-order results.\n\n<!--
START COPILOT CODING AGENT SUFFIX -->

- Fixes #1863

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com>

Author: Copilot

src/features/terminal/terminalEnvVarInjector.ts

@@ -29,6 +29,7 @@ export class TerminalEnvVarInjector implements Disposable {
     private disposables: Disposable[] = [];
     // Track which .env variables we've set for each workspace to avoid clearing shell activation variables
     private envVarKeys: Map<string, Set<string>> = new Map();
+    private refreshGenerations: Map<string, number> = new Map();
     private envFileNotificationShown = false;
 
     constructor(
@@ -49,19 +50,17 @@ export class TerminalEnvVarInjector implements Disposable {
             this.envVarManager.onDidChangeEnvironmentVariables((args) => {
                 if (!args.uri) {
                     // No specific URI, reload all workspaces
-                    this.updateEnvironmentVariables().catch((error) => {
+                    return this.updateEnvironmentVariables().catch((error) => {
                         traceError('Failed to update environment variables:', error);
                     });
-                    return;
                 }
 
                 const affectedWorkspace = getWorkspaceFolder(args.uri);
                 if (!affectedWorkspace) {
                     // No workspace folder found for this URI, reloading all workspaces
-                    this.updateEnvironmentVariables().catch((error) => {
+                    return this.updateEnvironmentVariables().catch((error) => {
                         traceError('Failed to update environment variables:', error);
                     });
-                    return;
                 }
 
                 // Check if env file injection is enabled when variables change
@@ -76,14 +75,9 @@ export class TerminalEnvVarInjector implements Disposable {
                     });
                 }
 
-                if (args.changeType === 2) {
-                    // FileChangeType.Deleted
-                    this.clearWorkspaceVariables(affectedWorkspace);
-                } else {
-                    this.updateEnvironmentVariables(affectedWorkspace).catch((error) => {
-                        traceError('Failed to update environment variables:', error);
-                    });
-                }
+                return this.updateEnvironmentVariables(affectedWorkspace).catch((error) => {
+                    traceError('Failed to update environment variables:', error);
+                });
             }),
         );
 
@@ -110,12 +104,17 @@ export class TerminalEnvVarInjector implements Disposable {
      */
     private async updateEnvironmentVariables(workspaceFolder?: WorkspaceFolder): Promise<void> {
         try {
+            let workspaceUpdates: { folder: WorkspaceFolder; generation: number }[];
             if (workspaceFolder) {
-                // Update only the specified workspace
+                workspaceUpdates = [
+                    {
+                        folder: workspaceFolder,
+                        generation: this.beginWorkspaceRefresh(workspaceFolder),
+                    },
+                ];
                 traceVerbose(
                     `TerminalEnvVarInjector: Updating environment variables for workspace: ${workspaceFolder.uri.fsPath}`,
                 );
-                await this.injectEnvironmentVariablesForWorkspace(workspaceFolder);
             } else {
                 // No provided workspace - update all workspaces
 
@@ -126,9 +125,14 @@ export class TerminalEnvVarInjector implements Disposable {
                 }
 
                 traceVerbose('TerminalEnvVarInjector: Updating environment variables for all workspaces');
-                for (const folder of workspaceFolders) {
-                    await this.injectEnvironmentVariablesForWorkspace(folder);
-                }
+                workspaceUpdates = workspaceFolders.map((folder) => ({
+                    folder,
+                    generation: this.beginWorkspaceRefresh(folder),
+                }));
+            }
+
+            for (const { folder, generation } of workspaceUpdates) {
+                await this.injectEnvironmentVariablesForWorkspace(folder, generation);
             }
 
             traceVerbose('TerminalEnvVarInjector: Environment variable injection completed');
@@ -140,7 +144,10 @@ export class TerminalEnvVarInjector implements Disposable {
     /**
      * Inject environment variables for a specific workspace.
      */
-    private async injectEnvironmentVariablesForWorkspace(workspaceFolder: WorkspaceFolder): Promise<void> {
+    private async injectEnvironmentVariablesForWorkspace(
+        workspaceFolder: WorkspaceFolder,
+        generation: number,
+    ): Promise<void> {
         const workspaceUri = workspaceFolder.uri;
         const workspaceKey = workspaceUri.fsPath;
 
@@ -159,6 +166,9 @@ export class TerminalEnvVarInjector implements Disposable {
                 traceVerbose(
                     `TerminalEnvVarInjector: Env file injection disabled for workspace: ${workspaceUri.fsPath}`,
                 );
+                if (!this.isCurrentWorkspaceRefresh(workspaceFolder, generation)) {
+                    return;
+                }
                 // Clear only the .env variables we previously set, not shell activation variables
                 this.clearTrackedEnvVariables(envVarScope, workspaceKey);
                 return;
@@ -170,8 +180,16 @@ export class TerminalEnvVarInjector implements Disposable {
                 : undefined;
             const defaultEnvFilePath: string = path.join(workspaceUri.fsPath, '.env');
 
-            let activeEnvFilePath: string = resolvedEnvFilePath || defaultEnvFilePath;
-            if (activeEnvFilePath && (await fse.pathExists(activeEnvFilePath))) {
+            const activeEnvFilePath =
+                resolvedEnvFilePath && (await fse.pathExists(resolvedEnvFilePath))
+                    ? resolvedEnvFilePath
+                    : (await fse.pathExists(defaultEnvFilePath))
+                      ? defaultEnvFilePath
+                      : undefined;
+            if (!this.isCurrentWorkspaceRefresh(workspaceFolder, generation)) {
+                return;
+            }
+            if (activeEnvFilePath) {
                 traceVerbose(`TerminalEnvVarInjector: Using env file: ${activeEnvFilePath}`);
             } else {
                 traceVerbose(
@@ -214,6 +232,17 @@ export class TerminalEnvVarInjector implements Disposable {
         }
     }
 
+    private beginWorkspaceRefresh(workspaceFolder: WorkspaceFolder): number {
+        const workspaceKey = workspaceFolder.uri.fsPath;
+        const generation = (this.refreshGenerations.get(workspaceKey) ?? 0) + 1;
+        this.refreshGenerations.set(workspaceKey, generation);
+        return generation;
+    }
+
+    private isCurrentWorkspaceRefresh(workspaceFolder: WorkspaceFolder, generation: number): boolean {
+        return this.refreshGenerations.get(workspaceFolder.uri.fsPath) === generation;
+    }
+
     /**
      * Offer to enable env file injection for the affected workspace folder or suppress future reminders.
      */
@@ -265,18 +294,6 @@ export class TerminalEnvVarInjector implements Disposable {
         return envVarCollection.getScoped(scope);
     }
 
-    /**
-     * Clear all environment variables for a workspace.
-     */
-    private clearWorkspaceVariables(workspaceFolder: WorkspaceFolder): void {
-        try {
-            const scope = this.getEnvironmentVariableCollectionScoped({ workspaceFolder });
-            scope.clear();
-        } catch (error) {
-            traceError(`Failed to clear environment variables for workspace ${workspaceFolder.uri.fsPath}:`, error);
-        }
-    }
-
     /**
      * Clear only the .env variables we've tracked, not shell activation variables.
      */

src/test/features/terminalEnvVarInjector.unit.test.ts

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 import * as assert from 'assert';
+import fse from 'fs-extra';
 import * as path from 'path';
 import * as sinon from 'sinon';
 import * as typeMoq from 'typemoq';
@@ -18,6 +19,7 @@ import {
 import { ActivationStrings, Common } from '../../common/localize';
 import * as logging from '../../common/logging';
 import * as persistentState from '../../common/persistentState';
+import { createDeferred } from '../../common/utils/deferred';
 import * as windowApis from '../../common/window.apis';
 import * as workspaceApis from '../../common/workspace.apis';
 import { EnvVarManager } from '../../features/execution/envVariableManager';
@@ -151,6 +153,131 @@ suite('TerminalEnvVarInjector', () => {
             assert.strictEqual(mockScopedCollection.replace.called, false);
         });
 
+        suite('env file changes', () => {
+            let envChangeCallback: ((args: { uri?: Uri; changeType: number }) => Promise<void>) | undefined;
+            let variables: Record<string, string>;
+            let variableReads: Promise<Record<string, string>>[];
+            let existingFiles: Set<string>;
+            const defaultEnvFile = path.join(testWorkspaceFolder.uri.fsPath, '.env');
+            const configuredEnvFile = path.join(testWorkspaceFolder.uri.fsPath, 'configured.env');
+
+            setup(() => {
+                variables = {};
+                variableReads = [];
+                existingFiles = new Set();
+                workspaceFoldersValue = undefined;
+                sinon
+                    .stub(fse, 'pathExists')
+                    .callsFake(async (filePath) => existingFiles.has(path.resolve(filePath.toString())));
+                sinon.stub(workspaceApis, 'getWorkspaceFolder').returns(testWorkspaceFolder);
+                envVarManager.reset();
+                envVarManager.setup((m) => m.onDidChangeEnvironmentVariables).returns(
+                    () => (listener) => {
+                        envChangeCallback = listener;
+                        return new Disposable(() => {});
+                    },
+                );
+                envVarManager
+                    .setup((m) => m.getEnvironmentVariables(typeMoq.It.isAny()))
+                    .returns(() => variableReads.shift() ?? Promise.resolve({ ...variables }));
+            });
+
+            async function fireChange(changeType: number, filePath = defaultEnvFile): Promise<void> {
+                assert.ok(envChangeCallback);
+                await envChangeCallback({ uri: Uri.file(filePath), changeType });
+            }
+
+            test('creating an env file with injection disabled preserves shell activation variables', async () => {
+                getConfigurationStub.returns(createMockConfig({ useEnvFile: false }) as WorkspaceConfiguration);
+                injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
+                await fireChange(2);
+
+                sinon.assert.notCalled(mockScopedCollection.clear);
+                sinon.assert.notCalled(mockScopedCollection.delete);
+                sinon.assert.notCalled(mockScopedCollection.replace);
+            });
+
+            test('creating, editing, and deleting an env file updates only its injected variables', async () => {
+                getConfigurationStub.returns(createMockConfig({ useEnvFile: true }) as WorkspaceConfiguration);
+                injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
+
+                existingFiles.add(defaultEnvFile);
+                variables = { TERMINAL_PROBE_VALUE: 'created' };
+                await fireChange(2);
+                sinon.assert.calledWith(mockScopedCollection.replace, 'TERMINAL_PROBE_VALUE', 'created');
+
+                variables = { OTHER_VALUE: 'edited' };
+                await fireChange(1);
+                sinon.assert.calledWith(mockScopedCollection.delete, 'TERMINAL_PROBE_VALUE');
+                sinon.assert.calledWith(mockScopedCollection.replace, 'OTHER_VALUE', 'edited');
+
+                existingFiles.delete(defaultEnvFile);
+                variables = {};
+                await fireChange(3);
+                sinon.assert.calledWith(mockScopedCollection.delete, 'OTHER_VALUE');
+                sinon.assert.notCalled(mockScopedCollection.clear);
+            });
+
+            test('deleting one env file retains variables from the other configured file', async () => {
+                getConfigurationStub.returns(
+                    createMockConfig({ useEnvFile: true, envFilePath: configuredEnvFile }) as WorkspaceConfiguration,
+                );
+                existingFiles.add(defaultEnvFile);
+                existingFiles.add(configuredEnvFile);
+                variables = { CONFIGURED_VALUE: 'configured', PROJECT_VALUE: 'project' };
+                injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
+                await fireChange(2, configuredEnvFile);
+
+                existingFiles.delete(configuredEnvFile);
+                variables = { PROJECT_VALUE: 'project' };
+                await fireChange(3, configuredEnvFile);
+
+                sinon.assert.calledWith(mockScopedCollection.delete, 'CONFIGURED_VALUE');
+                sinon.assert.neverCalledWith(mockScopedCollection.delete, 'PROJECT_VALUE');
+                sinon.assert.notCalled(mockScopedCollection.clear);
+            });
+
+            test('deleting the project env file retains variables from the configured file', async () => {
+                getConfigurationStub.returns(
+                    createMockConfig({ useEnvFile: true, envFilePath: configuredEnvFile }) as WorkspaceConfiguration,
+                );
+                existingFiles.add(defaultEnvFile);
+                existingFiles.add(configuredEnvFile);
+                variables = { CONFIGURED_VALUE: 'configured', PROJECT_VALUE: 'project' };
+                injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
+                await fireChange(2);
+
+                existingFiles.delete(defaultEnvFile);
+                variables = { CONFIGURED_VALUE: 'configured' };
+                await fireChange(3);
+
+                sinon.assert.calledWith(mockScopedCollection.delete, 'PROJECT_VALUE');
+                sinon.assert.neverCalledWith(mockScopedCollection.delete, 'CONFIGURED_VALUE');
+                sinon.assert.notCalled(mockScopedCollection.clear);
+            });
+
+            test('does not let an older env file refresh overwrite newer variables', async () => {
+                getConfigurationStub.returns(createMockConfig({ useEnvFile: true }) as WorkspaceConfiguration);
+                existingFiles.add(defaultEnvFile);
+                const olderRefresh = createDeferred<Record<string, string>>();
+                const newerRefresh = createDeferred<Record<string, string>>();
+                variableReads.push(olderRefresh.promise, newerRefresh.promise);
+                injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
+
+                assert.ok(envChangeCallback);
+                const firstChange = envChangeCallback({ uri: Uri.file(defaultEnvFile), changeType: 3 });
+                const secondChange = envChangeCallback({ uri: Uri.file(defaultEnvFile), changeType: 2 });
+
+                newerRefresh.resolve({ NEW_VALUE: 'new' });
+                await secondChange;
+                olderRefresh.resolve({ OLD_VALUE: 'old' });
+                await firstChange;
+
+                sinon.assert.calledOnceWithExactly(mockScopedCollection.replace, 'NEW_VALUE', 'new');
+                sinon.assert.neverCalledWith(mockScopedCollection.replace, 'OLD_VALUE', 'old');
+            });
+        });
+
         test('should NOT inject when useEnvFile is false even with python.envFile configured', async () => {
             getConfigurationStub.returns(
                 createMockConfig({