Prepend the active air executable location to the PATH of integrated terminals (#504)

* Prefer located `air` binary in integrated terminals

* CHANGELOG bullet

* Manage via LSP state changes and config changes

* Fix discriminated union

* Consistency

* Remove unused import

* Use correct camel case

Author: DavisVaughan

editors/code/CHANGELOG.md

@@ -7,6 +7,9 @@
 
 ## Development version
 
+- New `air.addExecutableToPath` configuration option to control whether the `air` executable located by `air.executableStrategy` is also added to the `PATH` of all integrated terminals. This ensures that a call to `air` in an integrated terminal uses the same version of Air used by the editor, which is particularly useful when agents call Air on your behalf. It is on by default (#500).
+
+
 ## 0.24.0
 
 - [Air 0.9.0](https://github.com/posit-dev/air/blob/main/CHANGELOG.md) is now bundled with the extension.

editors/code/package.json

@@ -83,7 +83,7 @@
 				},
 				"air.executableStrategy": {
 					"default": "bundled",
-					"markdownDescription": "Strategy used to locate the `air` executable to start the language server with.",
+					"markdownDescription": "Strategy used to locate the `air` executable.",
 					"enum": [
 						"bundled",
 						"environment",
@@ -102,6 +102,12 @@
 					"markdownDescription": "The path to an `air` executable. Only utilized when `air.executableStrategy` is set to \"path\".",
 					"scope": "window",
 					"type": "string"
+				},
+				"air.addExecutableToPath": {
+					"default": true,
+					"markdownDescription": "Whether to add the directory containing the `air` executable resolved by `air.executableStrategy` to the `PATH` of integrated terminals. This ensures that `air` invoked in an integrated terminal matches `air` used by the editor.",
+					"scope": "window",
+					"type": "boolean"
 				}
 			}
 		},

editors/code/src/environment.ts

@@ -0,0 +1,102 @@
+import * as vscode from "vscode";
+import * as path from "path";
+
+import * as output from "./output";
+import { getWorkspaceSettings } from "./settings";
+import { State, StateChange } from "./lsp";
+
+// Manages the `PATH` environment variable contribution we make to integrated
+// terminals.
+export class PathEnvironmentVariableManager implements vscode.Disposable {
+	private executableDirectory: string | null = null;
+	private workspaceFolder: vscode.WorkspaceFolder | null = null;
+
+	private subscriptions: vscode.Disposable[] = [];
+
+	constructor(
+		private collection: vscode.EnvironmentVariableCollection,
+		onStateChange: vscode.Event<StateChange>,
+	) {
+		// When the LSP fully starts up, sync our shared state and update `PATH` contribution.
+		// When the LSP shuts down (restart or deactivation), clear shared state and unset `PATH` contribution.
+		this.subscriptions.push(
+			onStateChange((change) => {
+				switch (change.state) {
+					case State.Started: {
+						this.executableDirectory = path.dirname(
+							change.binaryPath,
+						);
+						this.workspaceFolder = change.workspaceFolder;
+						this.update();
+						break;
+					}
+					case State.Stopped: {
+						this.executableDirectory = null;
+						this.workspaceFolder = null;
+						this.delete();
+						break;
+					}
+				}
+			}),
+		);
+
+		// When the user changes `air.addExecutableToPath`, reflect this immediately in new terminals
+		this.subscriptions.push(
+			vscode.workspace.onDidChangeConfiguration((event) => {
+				if (event.affectsConfiguration("air.addExecutableToPath")) {
+					this.update();
+				}
+			}),
+		);
+	}
+
+	private update(): void {
+		if (!this.workspaceFolder) {
+			return;
+		}
+		if (!this.executableDirectory) {
+			return;
+		}
+
+		const workspaceSettings = getWorkspaceSettings(
+			"air",
+			this.workspaceFolder,
+		);
+
+		if (workspaceSettings.addExecutableToPath) {
+			this.prepend(this.executableDirectory);
+		} else {
+			this.delete();
+		}
+	}
+
+	// Prepend a directory to the `PATH`
+	//
+	// `applyAtProcessCreation` and `applyAtShellIntegration` are really
+	// mutually exclusive:
+	//
+	// - `applyAtProcessCreation` would modify the `PATH` as early as possible,
+	//   before any shell startup scripts run (like zshrc, for example). This
+	//   could allow shell scripts to overwrite our modifications.
+	// - `applyAtShellIntegration` is the newer mechanism and applies the
+	//   modification via a VS Code managed shell integration script after any
+	//   shell startup scripts have run.
+	private prepend(directory: string): void {
+		this.collection.prepend("PATH", directory + path.delimiter, {
+			applyAtProcessCreation: false,
+			applyAtShellIntegration: true,
+		});
+		output.log(`Added \`${directory}\` to the terminal PATH.`);
+	}
+
+	private delete(): void {
+		this.collection.delete("PATH");
+		output.log(`Removed terminal PATH mutation.`);
+	}
+
+	dispose() {
+		for (const subscription of this.subscriptions) {
+			subscription.dispose();
+		}
+	}
+}

editors/code/src/extension.ts

@@ -2,11 +2,28 @@ import * as vscode from "vscode";
 import { Ctx } from "./context";
 import { Lsp } from "./lsp";
 import { registerCommands } from "./commands";
+import { PathEnvironmentVariableManager } from "./environment";
 
 let ctx: Ctx;
 
 export function activate(context: vscode.ExtensionContext) {
+	// Contributed environment variables should mirror the running server's
+	// resolved binary, should be recomputed on each start, and don't cache
+	// across IDE sessions. In particular, it would be bad if a collection was
+	// restored after an Air extension update, in which case there would be a
+	// short period where terminal `PATH`s point to an outdated location where
+	// an `air` executable no longer exists.
+	context.environmentVariableCollection.persistent = false;
+
 	let lsp = new Lsp(context);
+
+	context.subscriptions.push(
+		new PathEnvironmentVariableManager(
+			context.environmentVariableCollection,
+			lsp.onStateChange,
+		),
+	);
+
 	lsp.start();
 
 	ctx = new Ctx(context, lsp);

editors/code/src/lsp.ts

@@ -1,5 +1,6 @@
 import * as vscode from "vscode";
 import * as lc from "vscode-languageclient/node";
+import * as path from "path";
 import { default as PQueue } from "p-queue";
 import { getInitializationOptions, getWorkspaceSettings } from "./settings";
 import {
@@ -16,16 +17,29 @@ import { getRootWorkspaceFolder } from "./workspace";
 // concurrently and either result in a started or stopped state. Starting when
 // started is a noop, same for stopping when stopped. On the other hand
 // restarting is always scheduled.
-enum State {
+export enum State {
 	Started = "started",
 	Stopped = "stopped",
 }
 
+export interface StateChangeStarted {
+	state: State.Started;
+	binaryPath: string;
+	workspaceFolder: vscode.WorkspaceFolder;
+}
+export interface StateChangeStopped {
+	state: State.Stopped;
+}
+export type StateChange = StateChangeStarted | StateChangeStopped;
+
 export class Lsp {
 	private client: lc.LanguageClient | null = null;
 
 	private binaryPath: string | null = null;
 
+	public onStateChange: vscode.Event<StateChange>;
+	private onStateChangeEmitter: vscode.EventEmitter<StateChange>;
+
 	// We've received and processed an `air.toml` settings synchronization
 	// notification. Used to synchronize unit tests with the LSP.
 	private onSettingsNotification: vscode.Event<SyncFileSettingsParams>;
@@ -48,6 +62,10 @@ export class Lsp {
 		this.stateQueue = new PQueue({ concurrency: 1 });
 		this.fileSettings = new FileSettingsState(context);
 
+		this.onStateChangeEmitter = new vscode.EventEmitter<StateChange>();
+		context.subscriptions.push(this.onStateChangeEmitter);
+		this.onStateChange = this.onStateChangeEmitter.event;
+
 		this.onSettingsNotificationEmitter =
 			new vscode.EventEmitter<SyncFileSettingsParams>();
 		context.subscriptions.push(this.onSettingsNotificationEmitter);
@@ -204,6 +222,11 @@ export class Lsp {
 		this.client = client;
 		this.binaryPath = binaryPath;
 		this.state = State.Started;
+		this.onStateChangeEmitter.fire({
+			state: this.state,
+			binaryPath: this.binaryPath,
+			workspaceFolder: workspaceFolder,
+		});
 	}
 
 	private async stopImpl() {
@@ -221,6 +244,9 @@ export class Lsp {
 			this.state = State.Stopped;
 			this.client = null;
 			this.binaryPath = null;
+			this.onStateChangeEmitter.fire({
+				state: this.state,
+			});
 		}
 	}
 

editors/code/src/settings.ts

@@ -15,6 +15,7 @@ export type InitializationOptions = {
 export type WorkspaceSettings = {
 	executableStrategy: ExecutableStrategy;
 	executablePath?: string;
+	addExecutableToPath: boolean;
 };
 
 export function getInitializationOptions(
@@ -41,6 +42,7 @@ export function getWorkspaceSettings(
 		executableStrategy:
 			config.get<ExecutableStrategy>("executableStrategy") ?? "bundled",
 		executablePath: config.get<string>("executablePath"),
+		addExecutableToPath: config.get<boolean>("addExecutableToPath") ?? true,
 	};
 }