Merge remote-tracking branch 'origin/main' into feature/runtime-start…

…up-ordering

src/vs/workbench/api/browser/positron/mainThreadLanguageRuntime.ts

@@ -1177,11 +1177,15 @@ export class MainThreadLanguageRuntime
 		this._proxy = extHostContext.getProxy(ExtHostPositronContext.ExtHostLanguageRuntime);
 		this._id = MainThreadLanguageRuntime.MAX_ID++;
 
-		this._languageRuntimeService.onDidChangeRuntimeStartupPhase((phase) => {
-			if (phase === RuntimeStartupPhase.Discovering) {
-				this._proxy.$discoverLanguageRuntimes();
-			}
-		});
+		this._runtimeStartupService.registerMainThreadLanguageRuntime(this._id);
+
+		this._disposables.add(
+			this._languageRuntimeService.onDidChangeRuntimeStartupPhase((phase) => {
+				if (phase === RuntimeStartupPhase.Discovering) {
+					this._proxy.$discoverLanguageRuntimes();
+				}
+			})
+		);
 
 		this._disposables.add(this._runtimeSessionService.registerSessionManager(this));
 	}
@@ -1259,7 +1263,7 @@ export class MainThreadLanguageRuntime
 
 	// Signals that language runtime discovery is complete.
 	$completeLanguageRuntimeDiscovery(): void {
-		this._runtimeStartupService.completeDiscovery();
+		this._runtimeStartupService.completeDiscovery(this._id);
 	}
 
 	$unregisterLanguageRuntime(handle: number): void {
@@ -1289,6 +1293,8 @@ export class MainThreadLanguageRuntime
 				session.emitExit(exit);
 			}
 		});
+
+		this._runtimeStartupService.unregisterMainThreadLanguageRuntime(this._id);
 		this._disposables.dispose();
 	}
 

src/vs/workbench/services/runtimeSession/common/runtimeSessionService.ts

@@ -376,6 +376,16 @@ export interface IRuntimeSessionService {
 		runtimeMetadata: ILanguageRuntimeMetadata,
 		sessionId: string): Promise<boolean>;
 
+	/**
+	 * Validates a persisted runtime session before reconnecting to it.
+	 *
+	 * @param runtimeMetadata The metadata of the runtime.
+	 * @param sesionId The ID of the session to validate.
+	 */
+	validateRuntimeSession(
+		runtimeMetadata: ILanguageRuntimeMetadata,
+		sessionId: string): Promise<boolean>;
+
 	/**
 	 * Restores (reconnects to) a runtime session that was previously started.
 	 *

src/vs/workbench/services/runtimeStartup/common/runtimeStartup.ts

@@ -97,7 +97,13 @@ export class RuntimeStartupService extends Disposable implements IRuntimeStartup
 	// (metadata.languageId) of the runtime.
 	private readonly _mostRecentlyStartedRuntimesByLanguageId = new Map<string, ILanguageRuntimeMetadata>();
 
-	// The current startup phase an observeable value.
+	// A map of each extension host and its runtime discovery completion state.
+	// This is keyed by the the extension host's mainThreadLanguageRuntime's id
+	// This map is used to determine if runtime discovery has been completed
+	// across all extension hosts.
+	private readonly _discoveryCompleteByExtHostId = new Map<number, boolean>();
+
+	// The current startup phase
 	private _startupPhase: RuntimeStartupPhase;
 
 	// Whether we are shutting down
@@ -362,11 +368,66 @@ export class RuntimeStartupService extends Disposable implements IRuntimeStartup
 	}
 
 	/**
-	 * Completes the language runtime discovery phase. If no runtimes were
-	 * started or will be started, automatically start one.
+	/**
+	 * Signals that the runtime discovery phase is completed only after all
+	 * extension hosts have completed runtime discovery.
+	 *
+	 * If no runtimes were started or will be started, automatically start one.
 	 */
-	completeDiscovery(): void {
-		this.setStartupPhase(RuntimeStartupPhase.Complete);
+	public completeDiscovery(id: number): void {
+		// Update the extension host's runtime discovery state to 'Complete'
+		this._discoveryCompleteByExtHostId.set(id, true);
+		this._logService.debug(`[Runtime startup] Discovery completed for extension host with id: ${id}.`);
+
+		// Determine if all extension hosts have completed discovery
+		let discoveryCompletedByAllExtensionHosts = true;
+		for (const disoveryCompleted of this._discoveryCompleteByExtHostId.values()) {
+			if (!disoveryCompleted) {
+				discoveryCompletedByAllExtensionHosts = false;
+				break;
+			}
+		}
+
+		// The 'Discovery' phase is considered complete only after all extension hosts
+		// have signaled they have completed their own runtime discovery
+		if (discoveryCompletedByAllExtensionHosts) {
+			this.setStartupPhase(RuntimeStartupPhase.Complete);
+			// Reset the discovery state for each ext host so we are ready
+			// for possible re-discovery of runtimes
+			this._discoveryCompleteByExtHostId.forEach((_, extHostId, m) => {
+				m.set(extHostId, false);
+			});
+		}
+	}
+
+	/**
+	 * Used to register an instance of a MainThreadLanguageRuntime.
+	 *
+	 * This is required because there can be multiple extension hosts
+	 * and the startup service needs to know of all of them to track
+	 * the startup phase across all extension hosts.
+	 *
+	 * @param id The id of the MainThreadLanguageRuntime instance being registered.
+	 */
+	public registerMainThreadLanguageRuntime(id: number): void {
+		// Add the mainThreadLanguageRuntime instance id to the set of mainThreadLanguageRuntimes.
+		this._discoveryCompleteByExtHostId.set(id, false);
+		this._logService.debug(`[Runtime startup] Registered extension host with id: ${id}.`);
+	}
+
+	/**
+	 * Used to un-registers an instance of a MainThreadLanguageRuntime.
+	 *
+	 * This is required because there can be multiple extension hosts
+	 * and the startup service needs to know of all of them to track
+	 * the startup phase across all extension hosts.
+	 *
+	 * @param id The id of the MainThreadLanguageRuntime instance being un-registered.
+	 */
+	public unregisterMainThreadLanguageRuntime(id: number): void {
+		// Remove the mainThreadLanguageRuntime instance id to the set of mainThreadLanguageRuntimes.
+		this._discoveryCompleteByExtHostId.delete(id);
+		this._logService.debug(`[Runtime startup] Unregistered extension host with id: ${id}.`);
 	}
 
 	/**
@@ -826,6 +887,7 @@ export class RuntimeStartupService extends Disposable implements IRuntimeStartup
 	 * @param sessions The set of sessions to restore.
 	 */
 	private async restoreWorkspaceSessions(sessions: SerializedSessionMetadata[]) {
+
 		this.setStartupPhase(RuntimeStartupPhase.Reconnecting);
 
 		// Activate any extensions needed for the sessions that are persistent on the machine.

src/vs/workbench/services/runtimeStartup/common/runtimeStartupService.ts

@@ -43,8 +43,31 @@ export interface IRuntimeStartupService {
 	getAffiliatedRuntimeMetadata(languageId: string): ILanguageRuntimeMetadata | undefined;
 
 	/**
-	 * Signal that discovery of language runtimes is complete. Called from the
-	 * extension host.
+	 * Signal that discovery of language runtimes is completed for an extension host.
+	 *
+	 * @param id the id of the MainThreadLanguageRuntime instance for the extension host
+	 */
+	completeDiscovery(id: number): void;
+
+	/**
+	 * Used to register an instance of a MainThreadLanguageRuntime.
+	 *
+	 * This is required because there can be multiple extension hosts
+	 * and the startup service needs to know of all of them to track
+	 * the startup phase across all extension hosts.
+	 *
+	 * @param id The id of the MainThreadLanguageRuntime instance for the extension host.
+	 */
+	registerMainThreadLanguageRuntime(id: number): void;
+
+	/**
+	 * Used to un-register an instance of a MainThreadLanguageRuntime.
+	 *
+	 * This is required because there can be multiple extension hosts
+	 * and the startup service needs to know of all of them to track
+	 * the startup phase across all extension hosts.
+	 *
+	 * @param id The id of the MainThreadLanguageRuntime instance for the extension host.
 	 */
-	completeDiscovery(): void;
+	unregisterMainThreadLanguageRuntime(id: number): void;
 }

test/e2e/infra/fixtures/interpreter.ts

@@ -46,14 +46,8 @@ export class Interpreter {
 
 		await test.step(`Select interpreter via Quick Access: ${interpreterType}`, async () => {
 			interpreterType === 'Python'
-				? await this.console.selectInterpreter(InterpreterType.Python, DESIRED_PYTHON)
-				: await this.console.selectInterpreter(InterpreterType.R, DESIRED_R);
-
-			if (waitForReady) {
-				interpreterType === 'Python'
-					? await this.console.waitForReady('>>>', 30000)
-					: await this.console.waitForReady('>', 30000);
-			}
+				? await this.console.selectInterpreter(InterpreterType.Python, DESIRED_PYTHON, waitForReady)
+				: await this.console.selectInterpreter(InterpreterType.R, DESIRED_R, waitForReady);
 		});
 	};
 

test/e2e/tests/_test.setup.ts

@@ -109,13 +109,13 @@ export const test = base.extend<TestFixtures, WorkerFixtures>({
 	}, { scope: 'worker', auto: true, timeout: 60000 }],
 
 	interpreter: [async ({ app, page }, use) => {
-		const setInterpreter = async (desiredInterpreter: 'Python' | 'R') => {
+		const setInterpreter = async (desiredInterpreter: 'Python' | 'R', waitForReady = true) => {
 			const currentInterpreter = await page.locator('.top-action-bar-interpreters-manager').textContent() || '';
 
 			if (!currentInterpreter.startsWith(desiredInterpreter)) {
 				desiredInterpreter === 'Python'
-					? await app.workbench.interpreter.startInterpreterViaQuickAccess('Python')
-					: await app.workbench.interpreter.startInterpreterViaQuickAccess('R');
+					? await app.workbench.interpreter.startInterpreterViaQuickAccess('Python', waitForReady)
+					: await app.workbench.interpreter.startInterpreterViaQuickAccess('R', waitForReady);
 			}
 		};
 

test/e2e/tests/reticulate/reticulate.test.ts

@@ -19,10 +19,7 @@ test.describe('Reticulate', {
 }, () => {
 	test.beforeAll(async function ({ app, userSettings }) {
 		try {
-			// remove this once https://github.com/posit-dev/positron/issues/5226
-			// is resolved
 			await userSettings.set([
-				['kernelSupervisor.enable', 'false'],
 				['positron.reticulate.enabled', 'true']
 			]);
 
@@ -32,6 +29,10 @@ test.describe('Reticulate', {
 		}
 	});
 
+	// if running tests in sequence, we will need to skip waiting for ready because interpreters
+	// will already be running
+	let sequential = false;
+
 	test('R - Verify Basic Reticulate Functionality [C...]', async function ({ app, r, interpreter }) {
 
 		await app.workbench.console.pasteCodeToConsole('reticulate::repl_python()');
@@ -46,19 +47,69 @@ test.describe('Reticulate', {
 		}
 
 		await app.workbench.console.waitForReady('>>>');
-		await app.workbench.console.pasteCodeToConsole('x=100');
-		await app.workbench.console.sendEnterKey();
 
-		await interpreter.set('R');
+		await verifyReticulateFunctionality(app, interpreter, false);
 
-		await app.workbench.console.pasteCodeToConsole('y<-reticulate::py$x');
-		await app.workbench.console.sendEnterKey();
-		await app.workbench.layouts.enterLayout('fullSizedAuxBar');
+		sequential = true;
+
+	});
+
+	test('R - Verify Reticulate Stop/Restart Functionality [C...]', async function ({ app, interpreter }) {
+
+		// web only test but we don't have another way to skip electron tests
+		if (!app.web) {
+			return;
+		}
+
+		await app.workbench.interpreter.selectInterpreter('Python', 'Python (reticulate)', !sequential);
+
+		await verifyReticulateFunctionality(app, interpreter, sequential);
+
+		await app.workbench.layouts.enterLayout('stacked');
+
+		await app.workbench.console.barPowerButton.click();
+
+		await app.workbench.console.waitForConsoleContents('shut down successfully');
+
+		await app.code.driver.page.locator('.positron-console').getByRole('button', { name: 'Restart R' }).click();
+
+		await app.workbench.console.waitForReady('>');
 
-		await expect(async () => {
-			const variablesMap = await app.workbench.variables.getFlatVariables();
-			expect(variablesMap.get('y')).toStrictEqual({ value: '100', type: 'int' });
-		}).toPass({ timeout: 60000 });
+		await app.code.driver.page.locator('.positron-console').locator('.action-bar-button-drop-down-arrow').click();
+
+		await app.code.driver.page.locator('.action-label', { hasText: 'Python (reticulate)' }).hover();
+
+		await app.code.driver.page.keyboard.press('Enter');
+
+		await app.code.driver.page.locator('.positron-console').getByRole('button', { name: 'Restart Python' }).click();
+
+		await app.workbench.console.waitForReady('>>>');
+
+		await verifyReticulateFunctionality(app, interpreter, sequential);
 
 	});
 });
+
+async function verifyReticulateFunctionality(app, interpreter, sequential) {
+
+	await app.workbench.console.pasteCodeToConsole('x=100');
+	await app.workbench.console.sendEnterKey();
+
+	await app.workbench.console.barClearButton.click();
+
+	await interpreter.set('R', !sequential);
+
+	await app.workbench.console.pasteCodeToConsole('y<-reticulate::py$x');
+	await app.workbench.console.sendEnterKey();
+
+	await app.workbench.console.barClearButton.click();
+
+	await app.workbench.layouts.enterLayout('fullSizedAuxBar');
+
+	await expect(async () => {
+		const variablesMap = await app.workbench.variables.getFlatVariables();
+		expect(variablesMap.get('y')).toStrictEqual({ value: '100', type: 'int' });
+	}).toPass({ timeout: 60000 });
+
+	await app.workbench.layouts.enterLayout('stacked');
+};