Prepend PATH both at shell integration and process creation (#22905)

Author: Kartik Raj

src/client/terminals/envCollectionActivation/service.ts

@@ -130,7 +130,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
                     this.disposables,
                 );
                 const { shell } = this.applicationEnvironment;
-                const isActive = await this.shellIntegrationService.isWorking(shell);
+                const isActive = await this.shellIntegrationService.isWorking();
                 const shellType = identifyShellFromShellPath(shell);
                 if (!isActive && shellType !== TerminalShellType.commandPrompt) {
                     traceWarn(
@@ -218,6 +218,10 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
                         return;
                     }
                     if (key === 'PATH') {
+                        const options = {
+                            applyAtShellIntegration: true,
+                            applyAtProcessCreation: true,
+                        };
                         if (processEnv.PATH && env.PATH?.endsWith(processEnv.PATH)) {
                             // Prefer prepending to PATH instead of replacing it, as we do not want to replace any
                             // changes to PATH users might have made it in their init scripts (~/.bashrc etc.)
@@ -226,7 +230,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
                                 value = `${deactivate}${this.separator}${value}`;
                             }
                             traceVerbose(`Prepending environment variable ${key} in collection with ${value}`);
-                            envVarCollection.prepend(key, value, prependOptions);
+                            envVarCollection.prepend(key, value, options);
                         } else {
                             if (!value.endsWith(this.separator)) {
                                 value = value.concat(this.separator);
@@ -235,7 +239,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
                                 value = `${deactivate}${this.separator}${value}`;
                             }
                             traceVerbose(`Prepending environment variable ${key} in collection to ${value}`);
-                            envVarCollection.prepend(key, value, prependOptions);
+                            envVarCollection.prepend(key, value, options);
                         }
                         return;
                     }
@@ -300,7 +304,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
                 // PS1 should be set but no PS1 was set.
                 return;
             }
-            const config = await this.shellIntegrationService.isWorking(shell);
+            const config = await this.shellIntegrationService.isWorking();
             if (!config) {
                 traceVerbose('PS1 is not set when shell integration is disabled.');
                 return;
@@ -356,7 +360,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
     }
 
     private async getPrependOptions(): Promise<EnvironmentVariableMutatorOptions> {
-        const isActive = await this.shellIntegrationService.isWorking(this.applicationEnvironment.shell);
+        const isActive = await this.shellIntegrationService.isWorking();
         // Ideally we would want to prepend exactly once, either at shell integration or process creation.
         // TODO: Stop prepending altogether once https://github.com/microsoft/vscode/issues/145234 is available.
         return isActive

src/client/terminals/envCollectionActivation/shellIntegrationService.ts

@@ -97,7 +97,8 @@ export class ShellIntegrationService implements IShellIntegrationService {
 
     public readonly onDidChangeStatus = this.didChange.event;
 
-    public async isWorking(shell: string): Promise<boolean> {
+    public async isWorking(): Promise<boolean> {
+        const { shell } = this.appEnvironment;
         return this._isWorking(shell).catch((ex) => {
             traceError(`Failed to determine if shell supports shell integration`, shell, ex);
             return false;

src/client/terminals/types.ts

@@ -45,7 +45,7 @@ export interface ITerminalEnvVarCollectionService {
 export const IShellIntegrationService = Symbol('IShellIntegrationService');
 export interface IShellIntegrationService {
     onDidChangeStatus: Event<void>;
-    isWorking(shell: string): Promise<boolean>;
+    isWorking(): Promise<boolean>;
 }
 
 export const ITerminalDeactivateService = Symbol('ITerminalDeactivateService');

src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts

@@ -75,7 +75,7 @@ suite('Terminal Environment Variable Collection Service', () => {
         context = mock<IExtensionContext>();
         shell = mock<IApplicationShell>();
         shellIntegrationService = mock<IShellIntegrationService>();
-        when(shellIntegrationService.isWorking(anything())).thenResolve(true);
+        when(shellIntegrationService.isWorking()).thenResolve(true);
         globalCollection = mock<GlobalEnvironmentVariableCollection>();
         collection = mock<EnvironmentVariableCollection>();
         when(context.environmentVariableCollection).thenReturn(instance(globalCollection));
@@ -336,7 +336,7 @@ suite('Terminal Environment Variable Collection Service', () => {
         verify(collection.clear()).once();
         verify(collection.prepend('PATH', prependedPart, anything())).once();
         verify(collection.replace('PATH', anything(), anything())).never();
-        assert.deepEqual(opts, { applyAtProcessCreation: false, applyAtShellIntegration: true });
+        assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true });
     });
 
     test('Also prepend deactivate script location if available', async () => {
@@ -372,7 +372,7 @@ suite('Terminal Environment Variable Collection Service', () => {
         const separator = getOSType() === OSType.Windows ? ';' : ':';
         verify(collection.prepend('PATH', `scriptLocation${separator}${prependedPart}`, anything())).once();
         verify(collection.replace('PATH', anything(), anything())).never();
-        assert.deepEqual(opts, { applyAtProcessCreation: false, applyAtShellIntegration: true });
+        assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true });
     });
 
     test('Prepend full PATH with separator otherwise', async () => {
@@ -405,7 +405,7 @@ suite('Terminal Environment Variable Collection Service', () => {
         verify(collection.clear()).once();
         verify(collection.prepend('PATH', `${finalPath}${separator}`, anything())).once();
         verify(collection.replace('PATH', anything(), anything())).never();
-        assert.deepEqual(opts, { applyAtProcessCreation: false, applyAtShellIntegration: true });
+        assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true });
     });
 
     test('Prepend full PATH with separator otherwise', async () => {
@@ -441,7 +441,7 @@ suite('Terminal Environment Variable Collection Service', () => {
         verify(collection.clear()).once();
         verify(collection.prepend('PATH', `scriptLocation${separator}${finalPath}${separator}`, anything())).once();
         verify(collection.replace('PATH', anything(), anything())).never();
-        assert.deepEqual(opts, { applyAtProcessCreation: false, applyAtShellIntegration: true });
+        assert.deepEqual(opts, { applyAtProcessCreation: true, applyAtShellIntegration: true });
     });
 
     test('Verify envs are not applied if env activation is disabled', async () => {
@@ -523,7 +523,7 @@ suite('Terminal Environment Variable Collection Service', () => {
 
     test('Correct track that prompt was set for PS1 if shell integration is disabled', async () => {
         reset(shellIntegrationService);
-        when(shellIntegrationService.isWorking(anything())).thenResolve(false);
+        when(shellIntegrationService.isWorking()).thenResolve(false);
         when(platform.osType).thenReturn(OSType.Linux);
         const envVars: NodeJS.ProcessEnv = { VIRTUAL_ENV: 'prefix/to/venv', PS1: '(.venv)', ...process.env };
         const ps1Shell = 'bash';