diff --git a/extensions/positron-python/src/client/positron/runtime.ts b/extensions/positron-python/src/client/positron/runtime.ts index cd9eef255080..5795ec68bb5c 100644 --- a/extensions/positron-python/src/client/positron/runtime.ts +++ b/extensions/positron-python/src/client/positron/runtime.ts @@ -3,11 +3,9 @@ * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. *--------------------------------------------------------------------------------------------*/ -// eslint-disable-next-line import/no-unresolved import * as positron from 'positron'; import * as vscode from 'vscode'; import * as fs from 'fs-extra'; -import * as os from 'os'; import * as path from 'path'; import * as crypto from 'crypto'; @@ -250,13 +248,7 @@ export async function createPythonRuntimeMetadata( digest.update(pythonVersion); const runtimeId = digest.digest('hex').substring(0, 32); - // Create the runtime path. - // TODO@softwarenerd - We will need to update this for Windows. - const homedir = os.homedir(); - const runtimePath = - os.platform() !== 'win32' && interpreter.path.startsWith(homedir) - ? path.join('~', interpreter.path.substring(homedir.length)) - : interpreter.path; + const runtimePath = interpreter.path; // Save the ID of the Python environment for use when creating the language // runtime session. diff --git a/extensions/positron-r/src/provider.ts b/extensions/positron-r/src/provider.ts index e882e9c0333a..719759abeae4 100644 --- a/extensions/positron-r/src/provider.ts +++ b/extensions/positron-r/src/provider.ts @@ -642,11 +642,7 @@ export async function makeMetadata( const homedir = os.homedir(); const isUserInstallation = rInst.binpath.startsWith(homedir); - // Create the runtime path. - // TODO@softwarenerd - We will need to update this for Windows. - const runtimePath = os.platform() !== 'win32' && isUserInstallation ? - path.join('~', rInst.binpath.substring(homedir.length)) : - rInst.binpath; + const runtimePath = rInst.binpath; // Create the Rscript path. // The Rscript path is the same as the R binary path, but with the 'R' or 'R.exe' executable diff --git a/extensions/positron-r/src/runtime-quickpick.ts b/extensions/positron-r/src/runtime-quickpick.ts index a8ce14bcc517..0d2068188f3e 100644 --- a/extensions/positron-r/src/runtime-quickpick.ts +++ b/extensions/positron-r/src/runtime-quickpick.ts @@ -3,6 +3,8 @@ * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. *--------------------------------------------------------------------------------------------*/ +import * as os from 'os'; +import * as path from 'path'; import * as positron from 'positron'; import * as vscode from 'vscode'; import { rRuntimeDiscoverer, RRuntimeSource } from './provider'; @@ -18,7 +20,11 @@ class RuntimeQuickPickItem implements vscode.QuickPickItem { public runtimeMetadata: positron.LanguageRuntimeMetadata, ) { this.label = runtimeMetadata.runtimeName; - this.description = runtimeMetadata.runtimePath; + const homedir = os.homedir(); + this.description = + os.platform() !== 'win32' && runtimeMetadata.runtimePath.startsWith(homedir) + ? path.join('~', runtimeMetadata.runtimePath.substring(homedir.length)) + : runtimeMetadata.runtimePath; this.runtime = runtimeMetadata; } } diff --git a/src/positron-dts/positron.d.ts b/src/positron-dts/positron.d.ts index f0e6eec75204..3e7978feb25d 100644 --- a/src/positron-dts/positron.d.ts +++ b/src/positron-dts/positron.d.ts @@ -646,7 +646,11 @@ declare module 'positron' { * before the runtime is started. */ export interface LanguageRuntimeMetadata { - /** The path to the runtime. */ + /** + * The absolute path to the runtime executable. Always a fully-expanded + * path suitable for use with execFile or equivalent; never contains ~ + * or other shorthand. + */ runtimePath: string; /** A unique identifier for this runtime; takes the form of a GUID */ diff --git a/src/vs/workbench/browser/positronNewFolderFlow/utilities/interpreterDropDownUtils.ts b/src/vs/workbench/browser/positronNewFolderFlow/utilities/interpreterDropDownUtils.ts index 790f4fbac682..c0d249d2168a 100644 --- a/src/vs/workbench/browser/positronNewFolderFlow/utilities/interpreterDropDownUtils.ts +++ b/src/vs/workbench/browser/positronNewFolderFlow/utilities/interpreterDropDownUtils.ts @@ -6,7 +6,7 @@ import { DropDownListBoxEntry } from '../../positronComponents/dropDownListBox/dropDownListBox.js'; import { DropDownListBoxItem } from '../../positronComponents/dropDownListBox/dropDownListBoxItem.js'; import { DropDownListBoxSeparator } from '../../positronComponents/dropDownListBox/dropDownListBoxSeparator.js'; -import { ILanguageRuntimeMetadata } from '../../../services/languageRuntime/common/languageRuntimeService.js'; +import { getRuntimeDisplayPath, ILanguageRuntimeMetadata } from '../../../services/languageRuntime/common/languageRuntimeService.js'; /** * InterpreterInfo interface. @@ -52,7 +52,7 @@ export const interpretersToDropdownItems = ( runtimeId: runtime.runtimeId, languageName: runtime.languageName, languageVersion: runtime.languageVersion, - runtimePath: runtime.runtimePath, + runtimePath: getRuntimeDisplayPath(runtime), runtimeSource: runtime.runtimeSource, }, }) diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatRuntimeSessions.ts b/src/vs/workbench/contrib/chat/browser/actions/chatRuntimeSessions.ts index da6b68ddf550..d40a2869a1a9 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatRuntimeSessions.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatRuntimeSessions.ts @@ -9,7 +9,7 @@ import { URI } from '../../../../../base/common/uri.js'; import { localize } from '../../../../../nls.js'; import { ServicesAccessor } from '../../../../../platform/instantiation/common/instantiation.js'; import { IQuickInputService, IQuickPickItem, QuickPickItem } from '../../../../../platform/quickinput/common/quickInput.js'; -import { RuntimeState } from '../../../../services/languageRuntime/common/languageRuntimeService.js'; +import { getRuntimeDisplayPath, RuntimeState } from '../../../../services/languageRuntime/common/languageRuntimeService.js'; import { IRuntimeSessionService } from '../../../../services/runtimeSession/common/runtimeSessionService.js'; import { IChatWidget } from '../chat.js'; @@ -77,7 +77,7 @@ export async function showRuntimeSessionsPick(accessor: ServicesAccessor, _widge return { id: session.sessionId, label, - detail: session.runtimeMetadata.runtimePath, + detail: getRuntimeDisplayPath(session.runtimeMetadata), iconPath: { dark: URI.parse(`data:image/svg+xml;base64, ${session.runtimeMetadata.base64EncodedIconSvg}`), }, diff --git a/src/vs/workbench/contrib/languageRuntime/browser/languageRuntimeActions.ts b/src/vs/workbench/contrib/languageRuntime/browser/languageRuntimeActions.ts index 132953ca0090..518c374e8363 100644 --- a/src/vs/workbench/contrib/languageRuntime/browser/languageRuntimeActions.ts +++ b/src/vs/workbench/contrib/languageRuntime/browser/languageRuntimeActions.ts @@ -13,7 +13,7 @@ import { IQuickInputService, IQuickPickItem, QuickPickItem } from '../../../../p import { IKeybindingRule, KeybindingWeight } from '../../../../platform/keybinding/common/keybindingsRegistry.js'; import { LANGUAGE_RUNTIME_ACTION_CATEGORY } from '../common/languageRuntime.js'; import { IPositronConsoleService, POSITRON_CONSOLE_VIEW_ID } from '../../../services/positronConsole/browser/interfaces/positronConsoleService.js'; -import { ILanguageRuntimeMetadata, ILanguageRuntimeService, IRuntimePickerContribution, IRuntimePickerItem, LanguageRuntimeSessionMode, RuntimeCodeExecutionMode, RuntimeErrorBehavior, RuntimeStartupPhase, RuntimeState } from '../../../services/languageRuntime/common/languageRuntimeService.js'; +import { getRuntimeDisplayPath, ILanguageRuntimeMetadata, ILanguageRuntimeService, IRuntimePickerContribution, IRuntimePickerItem, LanguageRuntimeSessionMode, RuntimeCodeExecutionMode, RuntimeErrorBehavior, RuntimeStartupPhase, RuntimeState } from '../../../services/languageRuntime/common/languageRuntimeService.js'; import { ILanguageRuntimeSession, IRuntimeClientInstance, IRuntimeSessionService, RuntimeClientType, RuntimeStartMode } from '../../../services/runtimeSession/common/runtimeSessionService.js'; import { INotificationService } from '../../../../platform/notification/common/notification.js'; import { ILanguageService } from '../../../../editor/common/languages/language.js'; @@ -213,7 +213,7 @@ export const selectLanguageRuntimeSession = async ( .map(session => ({ id: session.sessionId, label: session.dynState.sessionName, - detail: session.runtimeMetadata.runtimePath, + detail: getRuntimeDisplayPath(session.runtimeMetadata), description: session.sessionId === currentForegroundSession?.sessionId ? localize('positron.languageRuntime.currentlySelected', 'Currently Selected') : undefined, @@ -242,7 +242,7 @@ export const selectLanguageRuntimeSession = async ( .map(session => ({ id: session.sessionId, label: getSessionDisplayNameWithRuntime(session), - detail: session.runtimeMetadata.runtimePath, + detail: getRuntimeDisplayPath(session.runtimeMetadata), description: session.sessionId === currentForegroundSession?.sessionId ? localize('positron.languageRuntime.currentlySelected', 'Currently Selected') : undefined, @@ -264,7 +264,7 @@ export const selectLanguageRuntimeSession = async ( .map(session => ({ id: session.sessionId, label: getSessionDisplayNameWithRuntime(session), - detail: session.runtimeMetadata.runtimePath, + detail: getRuntimeDisplayPath(session.runtimeMetadata), description: session.sessionId === currentForegroundSession?.sessionId ? localize('positron.languageRuntime.currentlySelected', 'Currently Selected') : undefined, @@ -474,7 +474,7 @@ export const selectNewLanguageRuntime = async ( items.push({ id: runtime.runtimeId, label: runtime.runtimeName, - detail: runtime.runtimePath, + detail: getRuntimeDisplayPath(runtime), iconPath: { dark: URI.parse(`data:image/svg+xml;base64, ${runtime.base64EncodedIconSvg}`), }, @@ -540,7 +540,7 @@ export const selectNewLanguageRuntime = async ( items.push({ id: runtime.runtimeId, label: runtime.runtimeName, - detail: runtime.runtimePath, + detail: getRuntimeDisplayPath(runtime), iconPath: { dark: URI.parse(`data:image/svg+xml;base64, ${runtime.base64EncodedIconSvg}`), }, @@ -899,7 +899,7 @@ export function registerLanguageRuntimeActions() { const runtimeQuickPickItems = runtimes.map(runtime => ({ id: runtime.runtimeId, label: `${runtime.languageName}: ${runtime.runtimeName}`, - description: runtime.runtimePath, + description: getRuntimeDisplayPath(runtime), runtime } satisfies LanguageRuntimeQuickPickItem)); diff --git a/src/vs/workbench/contrib/languageRuntime/test/browser/languageRuntimeActions.vitest.ts b/src/vs/workbench/contrib/languageRuntime/test/browser/languageRuntimeActions.vitest.ts index 509721b488ef..3ad3837d04da 100644 --- a/src/vs/workbench/contrib/languageRuntime/test/browser/languageRuntimeActions.vitest.ts +++ b/src/vs/workbench/contrib/languageRuntime/test/browser/languageRuntimeActions.vitest.ts @@ -87,12 +87,15 @@ describe('selectNewLanguageRuntime', () => { await vi.waitFor(() => expect(pick.show).toHaveBeenCalled()); } - function registerRuntime(metadata: ILanguageRuntimeMetadata): ILanguageRuntimeMetadata { - ctx.disposables.add(ctx.get(ILanguageRuntimeService).registerRuntime(metadata)); + async function registerRuntime(metadata: ILanguageRuntimeMetadata): Promise { + const runtimeService = ctx.get(ILanguageRuntimeService); + ctx.disposables.add(runtimeService.registerRuntime(metadata)); if (!preferredByLanguage.has(metadata.languageId)) { preferredByLanguage.set(metadata.languageId, metadata); } - return metadata; + // registerRuntime enriches the metadata into a new object (e.g. adds + // runtimeDisplayPath); return the stored instance the picker resolves to. + return runtimeService.getRegisteredRuntime(metadata.runtimeId) ?? metadata; } function pickItemById(id: string): IQuickPickItem | undefined { @@ -124,7 +127,7 @@ describe('selectNewLanguageRuntime', () => { }); it('resolves to the selected runtime metadata', async () => { - const py = registerRuntime(makeRuntime({ runtimeId: 'py-1' })); + const py = await registerRuntime(makeRuntime({ runtimeId: 'py-1' })); const promise = runPicker(); await waitUntilOpened(); const item = pickItemById('py-1')!; @@ -150,8 +153,8 @@ describe('selectNewLanguageRuntime', () => { describe('options.languageId', () => { it('filters runtimes to the given languageId', async () => { - registerRuntime(makeRuntime({ runtimeId: 'py-1', languageId: 'python', languageName: 'Python' })); - registerRuntime(makeRuntime({ runtimeId: 'r-1', languageId: 'r', languageName: 'R', runtimeName: 'R 4.4' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-1', languageId: 'python', languageName: 'Python' })); + await registerRuntime(makeRuntime({ runtimeId: 'r-1', languageId: 'r', languageName: 'R', runtimeName: 'R 4.4' })); const promise = runPicker({ languageId: 'python' }); await waitUntilOpened(); @@ -165,7 +168,7 @@ describe('selectNewLanguageRuntime', () => { }); it('passes languageId through to getPickerContributions', async () => { - registerRuntime(makeRuntime({ runtimeId: 'py-1' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-1' })); const spy = vi.spyOn(ctx.get(ILanguageRuntimeService), 'getPickerContributions'); const promise = runPicker({ languageId: 'python' }); await waitUntilOpened(); @@ -177,8 +180,8 @@ describe('selectNewLanguageRuntime', () => { describe('options.currentRuntimeId', () => { it('pre-focuses the matching item via activeItems', async () => { - registerRuntime(makeRuntime({ runtimeId: 'py-1' })); - registerRuntime(makeRuntime({ runtimeId: 'py-2', languageVersion: '3.10.0', runtimeName: 'Python 3.10' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-1' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-2', languageVersion: '3.10.0', runtimeName: 'Python 3.10' })); const promise = runPicker({ currentRuntimeId: 'py-2' }); await waitUntilOpened(); @@ -189,7 +192,7 @@ describe('selectNewLanguageRuntime', () => { }); it('leaves activeItems untouched when no item matches the id', async () => { - registerRuntime(makeRuntime({ runtimeId: 'py-1' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-1' })); const promise = runPicker({ currentRuntimeId: 'unknown-id' }); await waitUntilOpened(); expect(pick.activeItems).toEqual([]); @@ -200,8 +203,8 @@ describe('selectNewLanguageRuntime', () => { describe('item structure', () => { it('groups Suggested + per-environment-type runtimes with separators', async () => { - registerRuntime(makeRuntime({ runtimeId: 'py-system', runtimeSource: 'System', runtimeName: 'Python (System)' })); - registerRuntime(makeRuntime({ runtimeId: 'py-conda', runtimeSource: 'Conda', runtimeName: 'Python (Conda)' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-system', runtimeSource: 'System', runtimeName: 'Python (System)' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-conda', runtimeSource: 'Conda', runtimeName: 'Python (Conda)' })); const promise = runPicker(); await waitUntilOpened(); @@ -223,9 +226,9 @@ describe('selectNewLanguageRuntime', () => { }); it('sorts within an env type by version descending, unsupported runtimes last', async () => { - registerRuntime(makeRuntime({ runtimeId: 'py-310', languageVersion: '3.10.0', runtimeName: 'Python 3.10' })); - registerRuntime(makeRuntime({ runtimeId: 'py-312', languageVersion: '3.12.0', runtimeName: 'Python 3.12' })); - registerRuntime(makeRuntime({ + await registerRuntime(makeRuntime({ runtimeId: 'py-310', languageVersion: '3.10.0', runtimeName: 'Python 3.10' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-312', languageVersion: '3.12.0', runtimeName: 'Python 3.12' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-old', languageVersion: '3.8.0', runtimeName: 'Python 3.8 (unsupported)', extraRuntimeData: { supported: false }, })); @@ -246,21 +249,21 @@ describe('selectNewLanguageRuntime', () => { describe('reactive rebuild', () => { it('rebuilds when onDidRegisterRuntime fires mid-pick', async () => { - registerRuntime(makeRuntime({ runtimeId: 'py-1' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-1' })); const promise = runPicker(); await waitUntilOpened(); expect(pickItemById('py-1')).toBeDefined(); expect(pickItemById('py-2')).toBeUndefined(); - registerRuntime(makeRuntime({ runtimeId: 'py-2', languageVersion: '3.10.0' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-2', languageVersion: '3.10.0' })); expect(pickItemById('py-2')).toBeDefined(); pick.cancel(QuickInputHideReason.Gesture); await promise; }); it('rebuilds when onDidUnregisterRuntime fires mid-pick', async () => { - registerRuntime(makeRuntime({ runtimeId: 'py-1' })); - registerRuntime(makeRuntime({ runtimeId: 'py-2', languageVersion: '3.10.0' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-1' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-2', languageVersion: '3.10.0' })); const promise = runPicker(); await waitUntilOpened(); expect(pickItemById('py-1')).toBeDefined(); @@ -276,13 +279,13 @@ describe('selectNewLanguageRuntime', () => { }); it('preserves the previously focused item across rebuilds', async () => { - registerRuntime(makeRuntime({ runtimeId: 'py-1' })); - registerRuntime(makeRuntime({ runtimeId: 'py-2', languageVersion: '3.10.0' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-1' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-2', languageVersion: '3.10.0' })); const promise = runPicker({ currentRuntimeId: 'py-2' }); await waitUntilOpened(); expect(pick.activeItems[0].id).toBe('py-2'); - registerRuntime(makeRuntime({ runtimeId: 'py-3', languageVersion: '3.13.0' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-3', languageVersion: '3.13.0' })); expect(pick.activeItems[0].id).toBe('py-2'); pick.cancel(QuickInputHideReason.Gesture); await promise; @@ -291,7 +294,7 @@ describe('selectNewLanguageRuntime', () => { describe('startup phase', () => { it('re-fetches contributions when phase transitions to Complete', async () => { - registerRuntime(makeRuntime({ runtimeId: 'py-1' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-1' })); const runtimeService = ctx.get(ILanguageRuntimeService); runtimeService.setStartupPhase(RuntimeStartupPhase.Discovering); @@ -344,7 +347,7 @@ describe('selectNewLanguageRuntime', () => { it('shows a busy spinner and discovering placeholder while phase is not Complete', async () => { const runtimeService = ctx.get(ILanguageRuntimeService); runtimeService.setStartupPhase(RuntimeStartupPhase.Discovering); - registerRuntime(makeRuntime({ runtimeId: 'py-1' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-1' })); const promise = runPicker(); await waitUntilOpened(); @@ -372,7 +375,7 @@ describe('selectNewLanguageRuntime', () => { }); it('does not show a spinner when discovery is already complete on open', async () => { - registerRuntime(makeRuntime({ runtimeId: 'py-1' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-1' })); const promise = runPicker(); await waitUntilOpened(); expect(pick.busy).toBe(false); @@ -384,7 +387,7 @@ describe('selectNewLanguageRuntime', () => { it('toggles the spinner back on when phase leaves Complete while the picker is open', async () => { const runtimeService = ctx.get(ILanguageRuntimeService); - registerRuntime(makeRuntime({ runtimeId: 'py-1' })); + await registerRuntime(makeRuntime({ runtimeId: 'py-1' })); // beforeEach leaves the phase at Complete. const promise = runPicker(); await waitUntilOpened(); @@ -487,7 +490,9 @@ describe('selectNewLanguageRuntime', () => { const installItem = await waitForItemByLabel('Install Python via uv'); pick.accept(installItem); - await expect(promise).resolves.toEqual(installedRuntime); + // The picker resolves to the enriched, registered instance. + const result = await promise; + expect(result).toEqual(runtimeService.getRegisteredRuntime(installedRuntime.runtimeId)); expect(contribution.onSelect).toHaveBeenCalledWith('install-uv'); expect(rediscoverAllRuntimes).toHaveBeenCalledWith(/* quiet */ true); }); diff --git a/src/vs/workbench/contrib/positronAssistant/test/browser/positronAssistantService.vitest.ts b/src/vs/workbench/contrib/positronAssistant/test/browser/positronAssistantService.vitest.ts index 063a286055e6..4611ae09445a 100644 --- a/src/vs/workbench/contrib/positronAssistant/test/browser/positronAssistantService.vitest.ts +++ b/src/vs/workbench/contrib/positronAssistant/test/browser/positronAssistantService.vitest.ts @@ -66,7 +66,7 @@ describe('PositronAssistantService', () => { ctx.instantiationService.stub(IPositronPlotsService, ctx.disposables.add(createTestPlotsServiceWithPlots())); // Create test runtime sessions - const runtime = createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); + const runtime = await createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); testConsoleSession = await startTestLanguageRuntimeSession( ctx.instantiationService, ctx.disposables, diff --git a/src/vs/workbench/contrib/positronConsole/browser/components/consoleInstanceInfoButton.tsx b/src/vs/workbench/contrib/positronConsole/browser/components/consoleInstanceInfoButton.tsx index 17a484089368..a483182de585 100644 --- a/src/vs/workbench/contrib/positronConsole/browser/components/consoleInstanceInfoButton.tsx +++ b/src/vs/workbench/contrib/positronConsole/browser/components/consoleInstanceInfoButton.tsx @@ -21,6 +21,7 @@ import { ActionBarButton } from '../../../../../platform/positronActionBar/brows import { PositronModalPopup } from '../../../../browser/positronComponents/positronModalPopup/positronModalPopup.js'; import { PositronModalReactRenderer } from '../../../../../base/browser/positronModalReactRenderer.js'; import { ILanguageRuntimeSession, LanguageRuntimeSessionChannel } from '../../../../services/runtimeSession/common/runtimeSessionService.js'; +import { getRuntimeDisplayPath } from '../../../../services/languageRuntime/common/languageRuntimeService.js'; const positronConsoleInfo = localize('positron.console.info.label', "Console Information"); const localizeShowKernelOutputChannel = (channelName: string) => localize('positron.console.info.showKernelOutputChannel', "Show {0} Output Channel", channelName); @@ -153,7 +154,7 @@ const ConsoleInstanceInfoModalPopup = (props: ConsoleInstanceInfoModalPopupProps

{localize( 'positron.console.info.runtimePath', 'Path: {0}', - props.session.runtimeMetadata.runtimePath)} + getRuntimeDisplayPath(props.session.runtimeMetadata))}

{localize( 'positron.console.info.runtimeSource', 'Source: {0}', diff --git a/src/vs/workbench/contrib/positronConsole/browser/components/startupStatus.tsx b/src/vs/workbench/contrib/positronConsole/browser/components/startupStatus.tsx index c87f60fa9f65..521dbed7dff9 100644 --- a/src/vs/workbench/contrib/positronConsole/browser/components/startupStatus.tsx +++ b/src/vs/workbench/contrib/positronConsole/browser/components/startupStatus.tsx @@ -14,7 +14,7 @@ import { localize } from '../../../../../nls.js'; import { RuntimeStartupProgress } from './runtimeStartupProgress.js'; import { DisposableStore } from '../../../../../base/common/lifecycle.js'; import { ProgressBar } from '../../../../../base/browser/ui/progressbar/progressbar.js'; -import { RuntimeStartupPhase } from '../../../../services/languageRuntime/common/languageRuntimeService.js'; +import { getRuntimeDisplayPath, RuntimeStartupPhase } from '../../../../services/languageRuntime/common/languageRuntimeService.js'; import { IRuntimeAutoStartEvent } from '../../../../services/runtimeStartup/common/runtimeStartupService.js'; import { usePositronReactServicesContext } from '../../../../../base/browser/positronReactRendererContext.js'; import { EmbeddedLink } from '../../../../../base/browser/ui/positronComponents/embeddedLink/EmbeddedLink.js'; @@ -84,7 +84,7 @@ export const StartupStatus = () => { bar.worked(1); } if (runtime.runtimePath) { - setLatestRuntimePath(runtime.runtimePath); + setLatestRuntimePath(getRuntimeDisplayPath(runtime)); } })); diff --git a/src/vs/workbench/contrib/positronConsole/test/browser/startupStatus.vitest.tsx b/src/vs/workbench/contrib/positronConsole/test/browser/startupStatus.vitest.tsx index d48ec153a49f..8a3fd7ee5ac0 100644 --- a/src/vs/workbench/contrib/positronConsole/test/browser/startupStatus.vitest.tsx +++ b/src/vs/workbench/contrib/positronConsole/test/browser/startupStatus.vitest.tsx @@ -162,7 +162,7 @@ describe('StartupStatus', () => { rtl.render(); act(() => { - const runtime = stubInterface({ runtimePath: '/usr/bin/python' }); + const runtime = stubInterface({ runtimePath: '/usr/bin/python', runtimeDisplayPath: undefined }); langMock.registeredRuntimes.push(runtime); langMock.onDidRegisterRuntime.fire(runtime); }); @@ -174,7 +174,7 @@ describe('StartupStatus', () => { rtl.render(); act(() => { - const runtime = stubInterface({ runtimePath: '/opt/local/python' }); + const runtime = stubInterface({ runtimePath: '/opt/local/python', runtimeDisplayPath: undefined }); langMock.registeredRuntimes.push(runtime); langMock.onDidRegisterRuntime.fire(runtime); }); @@ -186,7 +186,7 @@ describe('StartupStatus', () => { rtl.render(); act(() => { - const runtime = stubInterface({ runtimePath: '/usr/bin/python' }); + const runtime = stubInterface({ runtimePath: '/usr/bin/python', runtimeDisplayPath: undefined }); langMock.registeredRuntimes.push(runtime); langMock.onDidRegisterRuntime.fire(runtime); }); diff --git a/src/vs/workbench/contrib/positronRuntimeSessions/browser/components/runtimeSessionCard.tsx b/src/vs/workbench/contrib/positronRuntimeSessions/browser/components/runtimeSessionCard.tsx index cd9433a4748f..e1e95aa0a975 100644 --- a/src/vs/workbench/contrib/positronRuntimeSessions/browser/components/runtimeSessionCard.tsx +++ b/src/vs/workbench/contrib/positronRuntimeSessions/browser/components/runtimeSessionCard.tsx @@ -11,7 +11,7 @@ import { useEffect, useState } from 'react'; // Other dependencies. import { ILanguageRuntimeSession } from '../../../../services/runtimeSession/common/runtimeSessionService.js'; -import { RuntimeExitReason, RuntimeState } from '../../../../services/languageRuntime/common/languageRuntimeService.js'; +import { getRuntimeDisplayPath, RuntimeExitReason, RuntimeState } from '../../../../services/languageRuntime/common/languageRuntimeService.js'; import { RuntimeClientList } from './runtimeClientList.js'; import { DisposableStore } from '../../../../../base/common/lifecycle.js'; @@ -85,7 +85,7 @@ export const RuntimeSessionCard = (props: runtimeSessionCardProps) => { {props.session.metadata.startReason}

- {props.session.runtimeMetadata.runtimePath} + {getRuntimeDisplayPath(props.session.runtimeMetadata)}
diff --git a/src/vs/workbench/contrib/positronStartupDiagnostics/browser/positronStartupDiagnosticsEditor.ts b/src/vs/workbench/contrib/positronStartupDiagnostics/browser/positronStartupDiagnosticsEditor.ts index f279dc6e907f..36ab688191aa 100644 --- a/src/vs/workbench/contrib/positronStartupDiagnostics/browser/positronStartupDiagnosticsEditor.ts +++ b/src/vs/workbench/contrib/positronStartupDiagnostics/browser/positronStartupDiagnosticsEditor.ts @@ -33,7 +33,7 @@ import { IRuntimeDiscoveryCache, RUNTIME_DISCOVERY_CACHE_ENABLED_SETTING } from '../../../services/runtimeStartup/common/runtimeDiscoveryCacheService.js'; -import { ILanguageRuntimeService } from '../../../services/languageRuntime/common/languageRuntimeService.js'; +import { getRuntimeDisplayPath, ILanguageRuntimeService } from '../../../services/languageRuntime/common/languageRuntimeService.js'; import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; import { IOutputService } from '../../../services/output/common/output.js'; import { IAdminPolicyService } from '../../../../platform/policy/common/adminPolicyService.js'; @@ -721,7 +721,7 @@ class PositronStartupDiagnosticsContentProvider implements ITextModelContentProv table.push([ runtime.extensionId.value, runtime.runtimeName, - runtime.runtimePath + getRuntimeDisplayPath(runtime) ]); } md.table(['Extension', 'Name', 'Path'], table); diff --git a/src/vs/workbench/contrib/positronWebviewPreloads/test/browser/positronWebviewPreloadReconciliation.vitest.ts b/src/vs/workbench/contrib/positronWebviewPreloads/test/browser/positronWebviewPreloadReconciliation.vitest.ts index cf7ba2d73f49..41396be0dd9c 100644 --- a/src/vs/workbench/contrib/positronWebviewPreloads/test/browser/positronWebviewPreloadReconciliation.vitest.ts +++ b/src/vs/workbench/contrib/positronWebviewPreloads/test/browser/positronWebviewPreloadReconciliation.vitest.ts @@ -233,7 +233,7 @@ describe('Positron - PositronWebviewPreloadService output reconciliation (#12887 it('reconciliation disposes an overlay but spares a widget sharing the cell', async () => { // Widgets need a live notebook session to be created. - const runtime = createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); + const runtime = await createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); const session = await startTestLanguageRuntimeSession(ctx.instantiationService, ctx.disposables, { runtime, notebookUri: NOTEBOOK_URI, diff --git a/src/vs/workbench/contrib/runtimeNotebookKernel/tests/browser/runtimeNotebookKernel.vitest.ts b/src/vs/workbench/contrib/runtimeNotebookKernel/tests/browser/runtimeNotebookKernel.vitest.ts index 3a6690aa5d3d..ad30eb5aa829 100644 --- a/src/vs/workbench/contrib/runtimeNotebookKernel/tests/browser/runtimeNotebookKernel.vitest.ts +++ b/src/vs/workbench/contrib/runtimeNotebookKernel/tests/browser/runtimeNotebookKernel.vitest.ts @@ -92,7 +92,7 @@ describe('Positron - RuntimeNotebookKernel', () => { })); // Create a test language runtime. - runtime = createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); + runtime = await createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); // Create the runtime notebook kernel. kernel = ctx.disposables.add(ctx.instantiationService.createInstance(RuntimeNotebookKernel, runtime)); @@ -573,7 +573,7 @@ describe('Positron - RuntimeNotebookKernel - executeCodeInCell', () => { })); // Create a test language runtime. - runtime = createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); + runtime = await createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); // Create the runtime notebook kernel. kernel = ctx.disposables.add(ctx.instantiationService.createInstance(RuntimeNotebookKernel, runtime)); diff --git a/src/vs/workbench/contrib/runtimeNotebookKernel/tests/browser/runtimeNotebookKernelService.vitest.ts b/src/vs/workbench/contrib/runtimeNotebookKernel/tests/browser/runtimeNotebookKernelService.vitest.ts index 50bee2600701..175b96417019 100644 --- a/src/vs/workbench/contrib/runtimeNotebookKernel/tests/browser/runtimeNotebookKernelService.vitest.ts +++ b/src/vs/workbench/contrib/runtimeNotebookKernel/tests/browser/runtimeNotebookKernelService.vitest.ts @@ -94,13 +94,13 @@ describe('Positron - RuntimeNotebookKernelService', () => { ctx.instantiationService.stub(IRuntimeNotebookKernelService, runtimeNotebookKernelService); // Create a test language runtime. - runtime = createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); + runtime = await createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); // Get the kernel corresponding to the test language runtime. kernel = runtimeNotebookKernelService.getKernelByRuntimeId(runtime.runtimeId)!; // Create another runtime and kernel to test swapping kernels. - anotherRuntime = createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); + anotherRuntime = await createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); anotherKernel = runtimeNotebookKernelService.getKernelByRuntimeId(anotherRuntime.runtimeId)!; // Register the 'test' language, otherwise cells can't be set to that language. @@ -110,7 +110,7 @@ describe('Positron - RuntimeNotebookKernelService', () => { it('kernel is added on language runtime registration', async () => { // Register a language runtime, and wait for the corresponding kernel to be added. const kernelPromise = Event.toPromise(notebookKernelService.onDidAddKernel); - const runtime = createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); + const runtime = await createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); const kernel = await kernelPromise; // Check the kernel's properties. diff --git a/src/vs/workbench/contrib/runtimeSession/test/browser/foregroundSessionContribution.vitest.ts b/src/vs/workbench/contrib/runtimeSession/test/browser/foregroundSessionContribution.vitest.ts index 111edce464ae..9a802f137a32 100644 --- a/src/vs/workbench/contrib/runtimeSession/test/browser/foregroundSessionContribution.vitest.ts +++ b/src/vs/workbench/contrib/runtimeSession/test/browser/foregroundSessionContribution.vitest.ts @@ -118,7 +118,7 @@ describe('Positron - ForegroundSessionContribution', () => { /** Start a console session and set it as foreground. */ async function startConsoleSession() { - const runtime = createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); + const runtime = await createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); const session = await startTestLanguageRuntimeSession( ctx.instantiationService, ctx.disposables, { runtime, @@ -132,7 +132,7 @@ describe('Positron - ForegroundSessionContribution', () => { /** Start a notebook session for the given URI. */ async function startNotebookSession(uri: URI = notebookUri) { - const runtime = createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); + const runtime = await createTestLanguageRuntimeMetadata(ctx.instantiationService, ctx.disposables); const session = await startTestLanguageRuntimeSession( ctx.instantiationService, ctx.disposables, { runtime, diff --git a/src/vs/workbench/services/languageRuntime/common/languageRuntime.ts b/src/vs/workbench/services/languageRuntime/common/languageRuntime.ts index 1b6a025e46c8..890a1488e8b4 100644 --- a/src/vs/workbench/services/languageRuntime/common/languageRuntime.ts +++ b/src/vs/workbench/services/languageRuntime/common/languageRuntime.ts @@ -5,6 +5,7 @@ import * as nls from '../../../../nls.js'; import { Event, Emitter } from '../../../../base/common/event.js'; +import { tildify } from '../../../../base/common/labels.js'; import { ILogService } from '../../../../platform/log/common/log.js'; import { Disposable, IDisposable, toDisposable } from '../../../../base/common/lifecycle.js'; import { InstantiationType, registerSingleton } from '../../../../platform/instantiation/common/extensions.js'; @@ -13,6 +14,7 @@ import { Registry } from '../../../../platform/registry/common/platform.js'; import { IConfigurationRegistry, Extensions as ConfigurationExtensions, ConfigurationScope, IConfigurationNode, } from '../../../../platform/configuration/common/configurationRegistry.js'; import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; import { ISettableObservable, observableValue } from '../../../../base/common/observable.js'; +import { IPathService } from '../../../services/path/common/pathService.js'; /** * The implementation of ILanguageRuntimeService @@ -38,6 +40,10 @@ export class LanguageRuntimeService extends Disposable implements ILanguageRunti // Map of picker contributions by handle private readonly _pickerContributions = new Map(); + // Cached user home path (remote-aware). Populated eagerly in the constructor + // so registerRuntime can run synchronously. + private _cachedUserHome: string | undefined; + //#endregion Private Properties //#region Constructor @@ -50,7 +56,8 @@ export class LanguageRuntimeService extends Disposable implements ILanguageRunti */ constructor( @ILogService private readonly _logService: ILogService, - @IConfigurationService private readonly _configurationService: IConfigurationService + @IConfigurationService private readonly _configurationService: IConfigurationService, + @IPathService private readonly _pathService: IPathService, ) { // Call the base class's constructor. super(); @@ -58,6 +65,14 @@ export class LanguageRuntimeService extends Disposable implements ILanguageRunti this._startupPhase = observableValue( 'runtime-startup-phase', RuntimeStartupPhase.Initializing); this.onDidChangeRuntimeStartupPhase = Event.fromObservable(this._startupPhase); + + // Kick off the remote-aware home resolution eagerly so it's likely + // cached by the time extensions call registerRuntime. If it hasn't + // resolved yet, registerRuntime skips tildification rather than + // falling back to the local-only path. + this._pathService.userHome({ preferLocal: false }).then(uri => { + this._cachedUserHome = uri.fsPath; + }); } /** @@ -131,11 +146,25 @@ export class LanguageRuntimeService extends Disposable implements ILanguageRunti `the '${metadata.languageId}' language is disabled.`); } + // Enrich metadata with a workbench-computed display path (~-shortened + // on non-Windows; absolute path unchanged on Windows or system paths). + // Preserve a caller-supplied runtimeDisplayPath; only compute when absent. + let runtimeDisplayPath = metadata.runtimeDisplayPath; + if (!runtimeDisplayPath && this._cachedUserHome) { + runtimeDisplayPath = tildify(metadata.runtimePath, this._cachedUserHome); + } + // If _cachedUserHome isn't ready yet, leave runtimeDisplayPath undefined; + // getRuntimeDisplayPath() falls back to the raw runtimePath. + const enriched: ILanguageRuntimeMetadata = { + ...metadata, + runtimeDisplayPath, + }; + // Add the runtime to the registered runtimes. - this._registeredRuntimesByRuntimeId.set(metadata.runtimeId, metadata); + this._registeredRuntimesByRuntimeId.set(enriched.runtimeId, enriched); // Signal that the set of registered runtimes has changed. - this._onDidRegisterRuntimeEmitter.fire(metadata); + this._onDidRegisterRuntimeEmitter.fire(enriched); // Logging. this._logService.trace(`Language runtime ${formatLanguageRuntimeMetadata(metadata)} successfully registered.`); diff --git a/src/vs/workbench/services/languageRuntime/common/languageRuntimeService.ts b/src/vs/workbench/services/languageRuntime/common/languageRuntimeService.ts index 61861d985278..462d50162709 100644 --- a/src/vs/workbench/services/languageRuntime/common/languageRuntimeService.ts +++ b/src/vs/workbench/services/languageRuntime/common/languageRuntimeService.ts @@ -27,6 +27,14 @@ export const formatLanguageRuntimeMetadata = (metadata: ILanguageRuntimeMetadata `name: ${metadata.runtimeName} ` + `version: ${metadata.languageVersion})`; +/** + * Returns the display path for a runtime, preferring the human-friendly + * runtimeDisplayPath (which may use ~ shorthand) over the raw runtimePath. + * Use this everywhere a path is shown to the user. + */ +export const getRuntimeDisplayPath = (metadata: Pick): string => + metadata.runtimeDisplayPath ?? metadata.runtimePath; + /** * Formats a language runtime session for logging. * @@ -911,9 +919,19 @@ export function signaturesEqual( * before the runtime is started. */ export interface ILanguageRuntimeMetadata { - /** The path to the kernel. */ + /** + * The absolute path to the runtime executable. Always a fully-expanded path + * suitable for use with execFile; never contains ~ or other shorthand. + */ readonly runtimePath: string; + /** + * The runtime path formatted for display to the user. May use ~ as + * shorthand for the user's home directory on non-Windows platforms. + * Falls back to runtimePath when not set. + */ + readonly runtimeDisplayPath?: string; + /** A unique identifier for this runtime; usually a GUID */ readonly runtimeId: string; diff --git a/src/vs/workbench/services/languageRuntime/test/common/languageRuntime.vitest.ts b/src/vs/workbench/services/languageRuntime/test/common/languageRuntime.vitest.ts index f687af8ff88f..03ab00ae3d2d 100644 --- a/src/vs/workbench/services/languageRuntime/test/common/languageRuntime.vitest.ts +++ b/src/vs/workbench/services/languageRuntime/test/common/languageRuntime.vitest.ts @@ -6,14 +6,28 @@ /// import { raceTimeout } from '../../../../../base/common/async.js'; +import { URI } from '../../../../../base/common/uri.js'; import { createTestContainer } from '../../../../../test/vitest/positronTestContainer.js'; import { stubInterface } from '../../../../../test/vitest/stubInterface.js'; import { ExtensionIdentifier } from '../../../../../platform/extensions/common/extensions.js'; import { ILogService, NullLogger } from '../../../../../platform/log/common/log.js'; import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js'; +import { IPathService } from '../../../../services/path/common/pathService.js'; import { LanguageRuntimeService } from '../../common/languageRuntime.js'; -import { ILanguageRuntimeMetadata, LanguageRuntimeSessionLocation, LanguageRuntimeStartupBehavior, LanguageStartupBehavior } from '../../common/languageRuntimeService.js'; +import { getRuntimeDisplayPath, ILanguageRuntimeMetadata, LanguageRuntimeSessionLocation, LanguageRuntimeStartupBehavior, LanguageStartupBehavior } from '../../common/languageRuntimeService.js'; + +const TEST_USER_HOME = URI.file('/home/testuser'); +const pathServiceStub = stubInterface({ + // Handle both overloads: preferLocal:true returns URI synchronously; + // preferLocal:false (or absent) returns Promise. + userHome: vi.fn().mockImplementation((options?: { preferLocal?: boolean }) => { + if (options?.preferLocal === true) { + return TEST_USER_HOME; + } + return Promise.resolve(TEST_USER_HOME); + }), +}); /** * Shared metadata fields for test stubs. Both tests use the same base shape; @@ -24,6 +38,7 @@ function makeTestMetadata(overrides: Partial): ILangua runtimeId: 'testRuntimeId', languageId: 'testLanguageId', runtimePath: '', + runtimeDisplayPath: undefined, languageName: 'testLanguage', languageVersion: '1.0.0', base64EncodedIconSvg: undefined, @@ -39,12 +54,23 @@ function makeTestMetadata(overrides: Partial): ILangua }); } +describe('getRuntimeDisplayPath', () => { + it('returns runtimeDisplayPath when set', () => { + expect(getRuntimeDisplayPath({ runtimePath: '/abs/path/R', runtimeDisplayPath: '~/bin/R' })).toBe('~/bin/R'); + }); + + it('falls back to runtimePath when runtimeDisplayPath is undefined', () => { + expect(getRuntimeDisplayPath({ runtimePath: '/abs/path/R', runtimeDisplayPath: undefined })).toBe('/abs/path/R'); + }); +}); + describe('Positron - LanguageRuntimeService', () => { describe('default configuration', () => { const ctx = createTestContainer() .withRuntimeServices() .stub(ILogService, new NullLogger()) .stub(IConfigurationService, new TestConfigurationService()) + .stub(IPathService, pathServiceStub) .build(); it('register and unregister a runtime', async () => { @@ -77,8 +103,10 @@ describe('Positron - LanguageRuntimeService', () => { await raceTimeout(didRegisterRuntime, 10, () => timedOut = true); expect(timedOut, 'Awaiting onDidRegisterRuntime event timed out').toBe(false); - // Check that the runtime was registered. - expect(languageRuntimeService.registeredRuntimes).toEqual([metadata]); + // Check that the runtime was registered (with workbench-computed display path). + expect(languageRuntimeService.registeredRuntimes).toMatchObject([ + { runtimeId: metadata.runtimeId, runtimePath: metadata.runtimePath }, + ]); // Unregister the runtime. languageRuntimeService.unregisterRuntime(metadata.runtimeId); @@ -89,6 +117,24 @@ describe('Positron - LanguageRuntimeService', () => { // No-op since we already unregistered the runtime. runtimeDisposable.dispose(); }); + + it('enriches runtimeDisplayPath via tildify on registration', async () => { + const languageRuntimeService = ctx.disposables.add(ctx.instantiationService.createInstance(LanguageRuntimeService)); + + // The constructor starts a userHome() promise; let the microtask run + // before calling registerRuntime so _cachedUserHome is populated. + await Promise.resolve(); + + // A path under the test user home should be tildified. + const underHome = makeTestMetadata({ runtimeId: 'tilde-1', runtimePath: '/home/testuser/bin/R' }); + languageRuntimeService.registerRuntime(underHome); + expect(languageRuntimeService.registeredRuntimes[0].runtimeDisplayPath).toBe('~/bin/R'); + + // A system path should be left unchanged. + const system = makeTestMetadata({ runtimeId: 'tilde-2', runtimePath: '/usr/bin/R' }); + languageRuntimeService.registerRuntime(system); + expect(languageRuntimeService.registeredRuntimes[1].runtimeDisplayPath).toBe('/usr/bin/R'); + }); }); describe('onDidUnregisterRuntime', () => { @@ -96,6 +142,7 @@ describe('Positron - LanguageRuntimeService', () => { .withRuntimeServices() .stub(ILogService, new NullLogger()) .stub(IConfigurationService, new TestConfigurationService()) + .stub(IPathService, pathServiceStub) .build(); it('fires with the runtimeId and removes the runtime when unregistered', () => { @@ -148,9 +195,10 @@ describe('Positron - LanguageRuntimeService', () => { .withRuntimeServices() .stub(ILogService, new NullLogger()) .stub(IConfigurationService, configService) + .stub(IPathService, pathServiceStub) .build(); - it('cannot register a runtime when the language is disabled in configuration', async () => { + it('cannot register a runtime when the language is disabled in configuration', () => { const languageRuntimeService = ctx.disposables.add(ctx.instantiationService.createInstance(LanguageRuntimeService)); // Create mock metadata for a runtime with the disabled language. @@ -160,9 +208,7 @@ describe('Positron - LanguageRuntimeService', () => { }); // Attempt to register the runtime - this should throw an error. - expect(() => { - languageRuntimeService.registerRuntime(metadata); - }).toThrow(); + expect(() => languageRuntimeService.registerRuntime(metadata)).toThrow(); // Verify that no runtimes were registered. expect(languageRuntimeService.registeredRuntimes.length).toBe(0); diff --git a/src/vs/workbench/services/runtimeSession/test/common/runtimeSession.vitest.ts b/src/vs/workbench/services/runtimeSession/test/common/runtimeSession.vitest.ts index e09894f67cd4..abe184fd147e 100644 --- a/src/vs/workbench/services/runtimeSession/test/common/runtimeSession.vitest.ts +++ b/src/vs/workbench/services/runtimeSession/test/common/runtimeSession.vitest.ts @@ -605,8 +605,10 @@ describe('Positron - RuntimeSessionService', () => { await restoreConsole(unregisteredRuntime); - // The runtime should now be registered. - expect(languageRuntimeService.getRegisteredRuntime(unregisteredRuntime.runtimeId)).toBe(unregisteredRuntime); + // The runtime should now be registered. registerRuntime enriches the + // metadata into a new object, so compare structurally rather than by + // reference. + expect(languageRuntimeService.getRegisteredRuntime(unregisteredRuntime.runtimeId)).toEqual(unregisteredRuntime); }); it('auto start validates runtime if unregistered', async () => { diff --git a/src/vs/workbench/services/runtimeSession/test/common/testRuntimeSessionService.ts b/src/vs/workbench/services/runtimeSession/test/common/testRuntimeSessionService.ts index e6b38ab923e6..9cfe37a0c561 100644 --- a/src/vs/workbench/services/runtimeSession/test/common/testRuntimeSessionService.ts +++ b/src/vs/workbench/services/runtimeSession/test/common/testRuntimeSessionService.ts @@ -96,7 +96,11 @@ export function createTestLanguageRuntimeMetadata( const manager = TestRuntimeSessionManager.instance; disposables.add(runtimeSessionService.registerSessionManager(manager)); - return runtime; + // Return the registered metadata instance rather than the local literal. + // registerRuntime enriches the metadata (e.g. adds runtimeDisplayPath) and + // stores that enriched object, which is what sessions reference; returning + // it keeps reference equality intact for callers. + return languageRuntimeService.getRegisteredRuntime(runtime.runtimeId) ?? runtime; } export interface IStartTestLanguageRuntimeSessionOptions { diff --git a/src/vs/workbench/services/runtimeStartup/common/runtimeDiscoveryCacheService.ts b/src/vs/workbench/services/runtimeStartup/common/runtimeDiscoveryCacheService.ts index 5fef64271407..aacb7906694f 100644 --- a/src/vs/workbench/services/runtimeStartup/common/runtimeDiscoveryCacheService.ts +++ b/src/vs/workbench/services/runtimeStartup/common/runtimeDiscoveryCacheService.ts @@ -60,8 +60,14 @@ export const RUNTIME_DISCOVERY_CACHE_REFRESH_INTERVAL_DAYS_DEFAULT = 1; * * v2 added per-bucket `discoveryRootSignature` so warm starts can detect newly * installed interpreters by re-statting the directories the extension scans. + * + * v3: `runtimePath` is now always the fully-expanded absolute path, with the + * `~` shorthand moved to the separate `runtimeDisplayPath` field. Entries + * written by older builds may still carry a `~`-shortened `runtimePath` and + * no `runtimeDisplayPath`; bumping the version forces those entries to be + * discarded and rediscovered rather than replayed as-is. */ -export const RUNTIME_DISCOVERY_CACHE_SCHEMA_VERSION = 2; +export const RUNTIME_DISCOVERY_CACHE_SCHEMA_VERSION = 3; /** * Storage key under which all cache state is persisted. Embeds the schema diff --git a/src/vs/workbench/services/runtimeStartup/common/runtimeStartup.ts b/src/vs/workbench/services/runtimeStartup/common/runtimeStartup.ts index fa0a72b1b27d..612f46d5fd49 100644 --- a/src/vs/workbench/services/runtimeStartup/common/runtimeStartup.ts +++ b/src/vs/workbench/services/runtimeStartup/common/runtimeStartup.ts @@ -578,7 +578,10 @@ export class RuntimeStartupService extends Disposable implements IRuntimeStartup for (const languageId of languageIds) { const metadata = this.getAffiliatedRuntimeMetadata(languageId); if (metadata) { - runtimes.push(metadata); + // Prefer the live registered metadata so callers always see + // fresh field values (e.g. runtimePath is always absolute). + const live = this._languageRuntimeService.getRegisteredRuntime(metadata.runtimeId); + runtimes.push(live ?? metadata); } } return runtimes; @@ -1598,6 +1601,11 @@ export class RuntimeStartupService extends Disposable implements IRuntimeStartup // If the runtime is affiliated with this workspace, start it. if (metadata.runtimeId === affiliatedRuntimeId) { + // Heal any stale fields in the stored affiliation (e.g. a runtimePath + // that was ~-shortened before the runtimePath/runtimeDisplayPath split) + // by overwriting with the freshly-discovered metadata. + this.saveAffiliatedRuntime({ ...affiliated, metadata }); + try { // Check the setting to see if we should be auto-starting. diff --git a/src/vs/workbench/services/runtimeStartup/test/common/runtimeStartup.vitest.ts b/src/vs/workbench/services/runtimeStartup/test/common/runtimeStartup.vitest.ts index 99e8c2efa0c0..1fcc29d72a31 100644 --- a/src/vs/workbench/services/runtimeStartup/test/common/runtimeStartup.vitest.ts +++ b/src/vs/workbench/services/runtimeStartup/test/common/runtimeStartup.vitest.ts @@ -33,6 +33,7 @@ import { BeforeShutdownEvent, ILifecycleService, WillShutdownEvent } from '../.. import { IPositronNewFolderService, NewFolderStartupPhase } from '../../../positronNewFolder/common/positronNewFolder.js'; import { ILanguageRuntimeSession } from '../../../runtimeSession/common/runtimeSessionService.js'; import { RuntimeStartupService } from '../../common/runtimeStartup.js'; +import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; import { ICachedRuntime, IDiscoveryCacheBucket, @@ -1043,6 +1044,72 @@ describe('Positron - RuntimeStartupService Architecture Mismatch', () => { }); }); +describe('RuntimeStartupService - affiliation healing', () => { + + const ctx = createTestContainer() + .withRuntimeServices() + .stub(IEphemeralStateService, { + getItem: () => Promise.resolve(undefined), + setItem: () => Promise.resolve(), + }) + .stub(ILifecycleService, { + onBeforeShutdown: new Emitter().event, + onWillShutdown: new Emitter().event, + }) + .stub(IPositronNewFolderService, { + onDidChangeNewFolderStartupPhase: new Emitter().event, + startupPhase: NewFolderStartupPhase.Complete, + }) + .stub(IProgressService, {}) + .stub(IWorkbenchEnvironmentService, { remoteAuthority: undefined }) + .stub(INotificationService, new TestNotificationService()) + .stub(IRuntimeDiscoveryCache, {}) + .build(); + + it('heals a stale ~-prefixed runtimePath in stored affiliation when the runtime registers', async () => { + const runtimeId = 'r-stale-path-test'; + const languageId = 'r'; + const staleRuntimePath = '~/local/lib/R/bin/R'; + const freshRuntimePath = '/home/positron/local/lib/R/bin/R'; + const storageKey = `positron.affiliatedRuntimeMetadata.v2.${languageId}`; + // TestContextService uses TestWorkspace (one folder, no configuration) → + // WorkbenchState.FOLDER → affiliationStorageScope() returns WORKSPACE. + const scope = StorageScope.WORKSPACE; + + // Pre-populate storage with a stale affiliation whose runtimePath was + // shortened with a ~ prefix before the runtimePath/runtimeDisplayPath split. + const staleAffiliated = { + metadata: metadata({ languageId, runtimePath: staleRuntimePath, runtimeId, extensionId: 'ms.r' }), + lastUsed: 0, + lastStarted: 0, + }; + ctx.get(IStorageService).store(storageKey, JSON.stringify(staleAffiliated), scope, StorageTarget.MACHINE); + + const svc = ctx.disposables.add( + ctx.instantiationService.createInstance(RuntimeStartupService)) as RuntimeStartupService; + // Force into LoadingCache phase so onDidRegisterRuntime heals the affiliation. + // LoadingCache (not Discovering) is used here to skip the upsert branch that + // only runs during Discovering; the heal runs in both phases. + (svc as unknown as { _startupPhase: RuntimeStartupPhase })._startupPhase = RuntimeStartupPhase.LoadingCache; + + // Use Manual startup behavior so the service heals the stored affiliation + // without attempting to auto-start a runtime (which has no extension host). + const freshMd: ILanguageRuntimeMetadata = { + ...metadata({ languageId, runtimePath: freshRuntimePath, runtimeId, extensionId: 'ms.r' }), + startupBehavior: LanguageRuntimeStartupBehavior.Manual, + }; + // Registering the runtime fires onDidRegisterRuntime, which heals the stored affiliation. + await ctx.get(ILanguageRuntimeService).registerRuntime(freshMd); + + // The stored affiliation must now carry the absolute path. + const stored = JSON.parse(ctx.get(IStorageService).get(storageKey, scope)!); + expect(stored.metadata.runtimePath).toBe(freshRuntimePath); + + // getAffiliatedRuntimes() returns the live registered metadata (absolute path). + expect(svc.getAffiliatedRuntimes()[0].runtimePath).toBe(freshRuntimePath); + }); +}); + /** * Mock notification service that captures prompt calls for testing. */ diff --git a/src/vs/workbench/test/common/positronWorkbenchTestServices.ts b/src/vs/workbench/test/common/positronWorkbenchTestServices.ts index a51ceceaf3d5..5c2f48d3bb7b 100644 --- a/src/vs/workbench/test/common/positronWorkbenchTestServices.ts +++ b/src/vs/workbench/test/common/positronWorkbenchTestServices.ts @@ -264,7 +264,10 @@ export class TestPathService implements IPathService { userHome(options: { preferLocal: true }): URI; userHome(options?: { preferLocal: boolean }): Promise; userHome(options?: { preferLocal: boolean }): URI | Promise { - return URI.file('/home/test'); + if (options?.preferLocal === true) { + return URI.file('/home/test'); + } + return Promise.resolve(URI.file('/home/test')); } hasValidBasename(resource: URI, basename?: string): Promise;