Apply env var collection path prefixes in shell integration scripts when mac+login shell

src/vs/platform/terminal/common/environmentVariable.ts

@@ -3,6 +3,8 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
+import { IProcessEnvironment } from 'vs/base/common/platform';
+
 export enum EnvironmentVariableMutatorType {
 	Replace = 1,
 	Append = 2,
@@ -12,8 +14,47 @@ export interface IEnvironmentVariableMutator {
 	readonly value: string;
 	readonly type: EnvironmentVariableMutatorType;
 }
+
+export interface IEnvironmentVariableCollection {
+	readonly map: ReadonlyMap<string, IEnvironmentVariableMutator>;
+}
+
 /** [variable, mutator] */
 export type ISerializableEnvironmentVariableCollection = [string, IEnvironmentVariableMutator][];
 
 /** [extension, collection] */
 export type ISerializableEnvironmentVariableCollections = [string, ISerializableEnvironmentVariableCollection][];
+
+export interface IExtensionOwnedEnvironmentVariableMutator extends IEnvironmentVariableMutator {
+	readonly extensionIdentifier: string;
+}
+
+export interface IMergedEnvironmentVariableCollectionDiff {
+	added: ReadonlyMap<string, IExtensionOwnedEnvironmentVariableMutator[]>;
+	changed: ReadonlyMap<string, IExtensionOwnedEnvironmentVariableMutator[]>;
+	removed: ReadonlyMap<string, IExtensionOwnedEnvironmentVariableMutator[]>;
+}
+
+type VariableResolver = (str: string) => Promise<string>;
+
+/**
+ * Represents an environment variable collection that results from merging several collections
+ * together.
+ */
+export interface IMergedEnvironmentVariableCollection {
+	readonly collections: ReadonlyMap<string, IEnvironmentVariableCollection>;
+	readonly map: ReadonlyMap<string, IExtensionOwnedEnvironmentVariableMutator[]>;
+
+	/**
+	 * Applies this collection to a process environment.
+	 * @param variableResolver An optional function to use to resolve variables within the
+	 * environment values.
+	 */
+	applyToProcessEnvironment(env: IProcessEnvironment, variableResolver?: VariableResolver): Promise<void>;
+
+	/**
+	 * Generates a diff of this collection against another. Returns undefined if the collections are
+	 * the same.
+	 */
+	diff(other: IMergedEnvironmentVariableCollection): IMergedEnvironmentVariableCollectionDiff | undefined;
+}

src/vs/workbench/contrib/terminal/common/environmentVariableCollection.ts → src/vs/platform/terminal/common/environmentVariableCollection.ts

@@ -4,8 +4,9 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { IProcessEnvironment, isWindows } from 'vs/base/common/platform';
-import { EnvironmentVariableMutatorType, IEnvironmentVariableCollection, IExtensionOwnedEnvironmentVariableMutator, IMergedEnvironmentVariableCollection, IMergedEnvironmentVariableCollectionDiff } from 'vs/workbench/contrib/terminal/common/environmentVariable';
-import { VariableResolver } from 'vs/workbench/contrib/terminal/common/terminalEnvironment';
+import { EnvironmentVariableMutatorType, IEnvironmentVariableCollection, IExtensionOwnedEnvironmentVariableMutator, IMergedEnvironmentVariableCollection, IMergedEnvironmentVariableCollectionDiff } from 'vs/platform/terminal/common/environmentVariable';
+
+type VariableResolver = (str: string) => Promise<string>;
 
 export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVariableCollection {
 	readonly map: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();

src/vs/workbench/contrib/terminal/common/environmentVariableShared.ts → src/vs/platform/terminal/common/environmentVariableShared.ts

@@ -3,7 +3,7 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { IEnvironmentVariableCollection, IEnvironmentVariableMutator, ISerializableEnvironmentVariableCollection, ISerializableEnvironmentVariableCollections } from 'vs/workbench/contrib/terminal/common/environmentVariable';
+import { IEnvironmentVariableCollection, IEnvironmentVariableMutator, ISerializableEnvironmentVariableCollection, ISerializableEnvironmentVariableCollections } from 'vs/platform/terminal/common/environmentVariable';
 
 // This file is shared between the renderer and extension host
 

src/vs/platform/terminal/node/terminalEnvironment.ts

@@ -7,14 +7,17 @@ import * as os from 'os';
 import { FileAccess } from 'vs/base/common/network';
 import { getCaseInsensitive } from 'vs/base/common/objects';
 import * as path from 'vs/base/common/path';
-import { IProcessEnvironment, isWindows } from 'vs/base/common/platform';
+import { IProcessEnvironment, isMacintosh, isWindows } from 'vs/base/common/platform';
 import * as process from 'vs/base/common/process';
 import { format } from 'vs/base/common/strings';
 import { isString } from 'vs/base/common/types';
 import * as pfs from 'vs/base/node/pfs';
 import { ILogService } from 'vs/platform/log/common/log';
 import { IProductService } from 'vs/platform/product/common/productService';
 import { IShellLaunchConfig, ITerminalEnvironment, ITerminalProcessOptions } from 'vs/platform/terminal/common/terminal';
+import { EnvironmentVariableMutatorType } from 'vs/platform/terminal/common/environmentVariable';
+import { deserializeEnvironmentVariableCollections } from 'vs/platform/terminal/common/environmentVariableShared';
+import { MergedEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariableCollection';
 
 export function getWindowsBuildNumber(): number {
 	const osVersion = (/(\d+)\.(\d+)\.(\d+)/g).exec(os.release());
@@ -104,7 +107,7 @@ export interface IShellIntegrationConfigInjection {
  */
 export function getShellIntegrationInjection(
 	shellLaunchConfig: IShellLaunchConfig,
-	options: Pick<ITerminalProcessOptions, 'shellIntegration' | 'windowsEnableConpty'>,
+	options: ITerminalProcessOptions,
 	env: ITerminalEnvironment | undefined,
 	logService: ILogService,
 	productService: IProductService
@@ -151,6 +154,7 @@ export function getShellIntegrationInjection(
 				newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.Bash);
 			} else if (areZshBashLoginArgs(originalArgs)) {
 				envMixin['VSCODE_SHELL_LOGIN'] = '1';
+				addEnvMixinPathPrefix(options, envMixin);
 				newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.Bash);
 			}
 			if (!newArgs) {
@@ -166,6 +170,7 @@ export function getShellIntegrationInjection(
 			const oldDataDirs = env?.XDG_DATA_DIRS ?? '/usr/local/share:/usr/share';
 			const newDataDir = path.join(appRoot, 'out/vs/workbench/contrib/xdg_data');
 			envMixin['XDG_DATA_DIRS'] = `${oldDataDirs}:${newDataDir}`;
+			addEnvMixinPathPrefix(options, envMixin);
 			return { newArgs: undefined, envMixin };
 		}
 		case 'pwsh': {
@@ -186,6 +191,7 @@ export function getShellIntegrationInjection(
 				newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.Zsh);
 			} else if (areZshBashLoginArgs(originalArgs)) {
 				newArgs = shellIntegrationArgs.get(ShellIntegrationExecutable.ZshLogin);
+				addEnvMixinPathPrefix(options, envMixin);
 			} else if (originalArgs === shellIntegrationArgs.get(ShellIntegrationExecutable.Zsh) || originalArgs === shellIntegrationArgs.get(ShellIntegrationExecutable.ZshLogin)) {
 				newArgs = originalArgs;
 			}
@@ -230,6 +236,39 @@ export function getShellIntegrationInjection(
 	return undefined;
 }
 
+/**
+ * On macOS the profile calls path_helper which adds a bunch of standard bin directories to the
+ * beginning of the PATH. This causes significant problems for the environment variable
+ * collection API as the custom paths added to the end will now be somewhere in the middle of
+ * the PATH. To combat this, VSCODE_PATH_PREFIX is used to re-apply any prefix after the profile
+ * has run. This will cause duplication in the PATH but should fix the issue.
+ *
+ * See #99878 for more information.
+ */
+function addEnvMixinPathPrefix(options: ITerminalProcessOptions, envMixin: IProcessEnvironment): void {
+	if (isMacintosh && options.environmentVariableCollections) {
+		// Deserialize and merge
+		const deserialized = deserializeEnvironmentVariableCollections(options.environmentVariableCollections);
+		const merged = new MergedEnvironmentVariableCollection(deserialized);
+
+		// Get all prepend PATH entries
+		const pathEntry = merged.map.get('PATH');
+		const prependToPath: string[] = [];
+		if (pathEntry) {
+			for (const mutator of pathEntry) {
+				if (mutator.type === EnvironmentVariableMutatorType.Prepend) {
+					prependToPath.push(mutator.value);
+				}
+			}
+		}
+
+		// Add to the environment mixin to be applied in the shell integration script
+		if (prependToPath.length > 0) {
+			envMixin['VSCODE_PATH_PREFIX'] = prependToPath.join('');
+		}
+	}
+}
+
 enum ShellIntegrationExecutable {
 	WindowsPwsh = 'windows-pwsh',
 	WindowsPwshLogin = 'windows-pwsh-login',

src/vs/platform/terminal/node/terminalProcess.ts

@@ -202,7 +202,7 @@ export class TerminalProcess extends Disposable implements ITerminalChildProcess
 
 		let injection: IShellIntegrationConfigInjection | undefined;
 		if (this._options.shellIntegration.enabled) {
-			injection = getShellIntegrationInjection(this.shellLaunchConfig, { shellIntegration: this._options.shellIntegration, windowsEnableConpty: this._options.windowsEnableConpty }, this._ptyOptions.env, this._logService, this._productService);
+			injection = getShellIntegrationInjection(this.shellLaunchConfig, this._options, this._ptyOptions.env, this._logService, this._productService);
 			if (injection) {
 				this._onDidChangeProperty.fire({ type: ProcessPropertyType.UsedShellIntegrationInjection, value: true });
 				if (injection.envMixin) {

src/vs/platform/terminal/test/node/terminalEnvironment.test.ts

@@ -11,9 +11,9 @@ import { IProductService } from 'vs/platform/product/common/productService';
 import { ITerminalProcessOptions } from 'vs/platform/terminal/common/terminal';
 import { getShellIntegrationInjection, IShellIntegrationConfigInjection } from 'vs/platform/terminal/node/terminalEnvironment';
 
-const enabledProcessOptions: Pick<ITerminalProcessOptions, 'shellIntegration' | 'windowsEnableConpty'> = { shellIntegration: { enabled: true }, windowsEnableConpty: true };
-const disabledProcessOptions: Pick<ITerminalProcessOptions, 'shellIntegration' | 'windowsEnableConpty'> = { shellIntegration: { enabled: false }, windowsEnableConpty: true };
-const winptyProcessOptions: Pick<ITerminalProcessOptions, 'shellIntegration' | 'windowsEnableConpty'> = { shellIntegration: { enabled: true }, windowsEnableConpty: false };
+const enabledProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: true }, windowsEnableConpty: true, environmentVariableCollections: undefined };
+const disabledProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: false }, windowsEnableConpty: true, environmentVariableCollections: undefined };
+const winptyProcessOptions: ITerminalProcessOptions = { shellIntegration: { enabled: true }, windowsEnableConpty: false, environmentVariableCollections: undefined };
 const pwshExe = process.platform === 'win32' ? 'pwsh.exe' : 'pwsh';
 const repoRoot = process.platform === 'win32' ? process.cwd()[0].toLowerCase() + process.cwd().substring(1) : process.cwd();
 const logService = new NullLogService();

src/vs/server/node/remoteTerminalChannel.ts

@@ -20,9 +20,9 @@ import { IGetTerminalLayoutInfoArgs, ISetTerminalLayoutInfoArgs } from 'vs/platf
 import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace';
 import { createURITransformer } from 'vs/workbench/api/node/uriTransformer';
 import { CLIServerBase, ICommandsExecuter } from 'vs/workbench/api/node/extHostCLIServer';
-import { IEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable';
-import { MergedEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariableCollection';
-import { deserializeEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariableShared';
+import { IEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariable';
+import { MergedEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariableCollection';
+import { deserializeEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariableShared';
 import { ICreateTerminalProcessArguments, ICreateTerminalProcessResult, IWorkspaceFolderData } from 'vs/workbench/contrib/terminal/common/remoteTerminalChannel';
 import * as terminalEnvironment from 'vs/workbench/contrib/terminal/common/terminalEnvironment';
 import { AbstractVariableResolverService } from 'vs/workbench/services/configurationResolver/common/variableResolver';

src/vs/workbench/api/browser/mainThreadTerminalService.ts

@@ -14,8 +14,8 @@ import { IProcessProperty, IShellLaunchConfig, IShellLaunchConfigDto, ProcessPro
 import { TerminalDataBufferer } from 'vs/platform/terminal/common/terminalDataBuffering';
 import { ITerminalEditorService, ITerminalExternalLinkProvider, ITerminalGroupService, ITerminalInstance, ITerminalLink, ITerminalService } from 'vs/workbench/contrib/terminal/browser/terminal';
 import { TerminalProcessExtHostProxy } from 'vs/workbench/contrib/terminal/browser/terminalProcessExtHostProxy';
-import { IEnvironmentVariableService, ISerializableEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable';
-import { deserializeEnvironmentVariableCollection, serializeEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariableShared';
+import { IEnvironmentVariableService } from 'vs/workbench/contrib/terminal/common/environmentVariable';
+import { deserializeEnvironmentVariableCollection, serializeEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariableShared';
 import { IStartExtensionTerminalRequest, ITerminalProcessExtHostProxy, ITerminalProfileResolverService, ITerminalProfileService, ITerminalQuickFixService } from 'vs/workbench/contrib/terminal/common/terminal';
 import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService';
 import { withNullAsUndefined } from 'vs/base/common/types';
@@ -26,6 +26,7 @@ import { CancellationToken } from 'vs/base/common/cancellation';
 import { ITerminalCommand } from 'vs/platform/terminal/common/capabilities/capabilities';
 import { ITerminalOutputMatch, ITerminalOutputMatcher, ITerminalQuickFix, ITerminalQuickFixOptions } from 'vs/platform/terminal/common/xterm/terminalQuickFix';
 import { TerminalQuickFixType } from 'vs/workbench/api/common/extHostTypes';
+import { ISerializableEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariable';
 
 
 @extHostNamedCustomer(MainContext.MainThreadTerminalService)

src/vs/workbench/api/common/extHost.protocol.ts

@@ -55,7 +55,7 @@ import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange';
 import { OutputChannelUpdateMode } from 'vs/workbench/services/output/common/output';
 import { InputValidationType } from 'vs/workbench/contrib/scm/common/scm';
 import { IWorkspaceSymbol } from 'vs/workbench/contrib/search/common/search';
-import { ISerializableEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable';
+import { ISerializableEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariable';
 import { CoverageDetails, ExtensionRunTestsRequest, ICallProfileRunHandler, IFileCoverage, ISerializedTestResults, ITestItem, ITestMessage, ITestRunProfile, ITestRunTask, ResolvedTestRunRequest, IStartControllerTests, TestResultState, TestsDiffOp } from 'vs/workbench/contrib/testing/common/testTypes';
 import { Timeline, TimelineChangeEvent, TimelineOptions, TimelineProviderDescriptor } from 'vs/workbench/contrib/timeline/common/timeline';
 import { TypeHierarchyItem } from 'vs/workbench/contrib/typeHierarchy/common/typeHierarchy';

src/vs/workbench/api/common/extHostTerminalService.ts

@@ -14,10 +14,10 @@ import { Disposable as VSCodeDisposable, EnvironmentVariableMutatorType, Termina
 import { IExtensionDescription } from 'vs/platform/extensions/common/extensions';
 import { localize } from 'vs/nls';
 import { NotSupportedError } from 'vs/base/common/errors';
-import { serializeEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariableShared';
+import { serializeEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariableShared';
 import { CancellationTokenSource } from 'vs/base/common/cancellation';
 import { generateUuid } from 'vs/base/common/uuid';
-import { ISerializableEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable';
+import { ISerializableEnvironmentVariableCollection } from 'vs/platform/terminal/common/environmentVariable';
 import { ICreateContributedTerminalProfileOptions, IProcessReadyEvent, IShellLaunchConfigDto, ITerminalChildProcess, ITerminalLaunchError, ITerminalProfile, TerminalIcon, TerminalLocation, IProcessProperty, ProcessPropertyType, IProcessPropertyMap } from 'vs/platform/terminal/common/terminal';
 import { TerminalDataBufferer } from 'vs/platform/terminal/common/terminalDataBuffering';
 import { ThemeColor } from 'vs/platform/theme/common/themeService';

src/vs/workbench/contrib/terminal/browser/environmentVariableInfo.ts

@@ -3,13 +3,14 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { EnvironmentVariableMutatorType, IEnvironmentVariableInfo, IMergedEnvironmentVariableCollection, IMergedEnvironmentVariableCollectionDiff } from 'vs/workbench/contrib/terminal/common/environmentVariable';
+import { IEnvironmentVariableInfo } from 'vs/workbench/contrib/terminal/common/environmentVariable';
 import { TerminalCommandId } from 'vs/workbench/contrib/terminal/common/terminal';
 import { ITerminalService } from 'vs/workbench/contrib/terminal/browser/terminal';
 import { localize } from 'vs/nls';
 import { ThemeIcon } from 'vs/platform/theme/common/themeService';
 import { Codicon } from 'vs/base/common/codicons';
 import { IHoverAction } from 'vs/workbench/services/hover/browser/hover';
+import { EnvironmentVariableMutatorType, IMergedEnvironmentVariableCollection, IMergedEnvironmentVariableCollectionDiff } from 'vs/platform/terminal/common/environmentVariable';
 
 export class EnvironmentVariableInfoStale implements IEnvironmentVariableInfo {
 	readonly requiresAction = true;

src/vs/workbench/contrib/terminal/browser/media/shellIntegration-bash.sh

@@ -31,6 +31,12 @@ if [ "$VSCODE_INJECTION" == "1" ]; then
 			. ~/.profile
 		fi
 		builtin unset VSCODE_SHELL_LOGIN
+
+		# Apply any explicit path prefix (see #99878)
+		if [ -n "$VSCODE_PATH_PREFIX" ]; then
+			export PATH=$VSCODE_PATH_PREFIX$PATH
+			builtin unset VSCODE_PATH_PREFIX
+		fi
 	fi
 	builtin unset VSCODE_INJECTION
 fi

src/vs/workbench/contrib/terminal/browser/media/shellIntegration-profile.zsh

@@ -7,4 +7,10 @@ if [[ $options[norcs] = off && -o "login" &&  -f $USER_ZDOTDIR/.zprofile ]]; the
 	ZDOTDIR=$USER_ZDOTDIR
 	. $USER_ZDOTDIR/.zprofile
 	ZDOTDIR=$VSCODE_ZDOTDIR
+
+	# Apply any explicit path prefix (see #99878)
+	if (( ${+VSCODE_PATH_PREFIX} )); then
+		export PATH=$VSCODE_PATH_PREFIX$PATH
+	fi
+	builtin unset VSCODE_PATH_PREFIX
 fi

src/vs/workbench/contrib/terminal/browser/media/shellIntegration.fish

@@ -22,6 +22,12 @@ or exit
 
 set --global VSCODE_SHELL_INTEGRATION 1
 
+# Apply any explicit path prefix (see #99878)
+if status --is-login; and set -q VSCODE_PATH_PREFIX
+	fish_add_path -p $VSCODE_PATH_PREFIX
+end
+set -e VSCODE_PATH_PREFIX
+
 # Helper function
 function __vsc_esc -d "Emit escape sequences for VS Code shell integration"
 	builtin printf "\e]633;%s\a" (string join ";" $argv)