Skip to content

Commit a838fb7

Browse files
austin3dickeyaustin3dickey
committed
wip
1 parent 4f4eb44 commit a838fb7

File tree

9 files changed

+214
-16
lines changed

9 files changed

+214
-16
lines changed

‎extensions/positron-supervisor/src/KallichoreSession.ts

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -330,18 +330,21 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession {
330330
this._kernelSpec = kernelSpec;
331331
const varActions = await this.buildEnvVarActions(false);
332332

333-
// Prepare the working directory; use the workspace root if available,
334-
// otherwise the home directory
335-
let workingDir = vscode.workspace.workspaceFolders?.[0].uri.fsPath || os.homedir();
336-
337-
// If we have a notebook URI, use its parent directory as the working
338-
// directory instead. Note that not all notebooks have valid on-disk
339-
// URIs since they may be transient or not yet saved; for these, we fall
340-
// back to the workspace root or home directory.
341-
if (this.metadata.notebookUri?.fsPath) {
342-
const notebookPath = this.metadata.notebookUri.fsPath;
343-
if (fs.existsSync(notebookPath)) {
344-
workingDir = path.dirname(notebookPath);
333+
let workingDir = this.metadata.workingDirectory;
334+
335+
if (!workingDir) {
336+
// Use the workspace root if available, otherwise the home directory
337+
workingDir = vscode.workspace.workspaceFolders?.[0].uri.fsPath || os.homedir();
338+
339+
// If we have a notebook URI, use its parent directory as the working
340+
// directory instead. Note that not all notebooks have valid on-disk
341+
// URIs since they may be transient or not yet saved; for these, we fall
342+
// back to the workspace root or home directory.
343+
if (this.metadata.notebookUri?.fsPath) {
344+
const notebookPath = this.metadata.notebookUri.fsPath;
345+
if (fs.existsSync(notebookPath)) {
346+
workingDir = path.dirname(notebookPath);
347+
}
345348
}
346349
}
347350

‎src/positron-dts/positron.d.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,9 @@ declare module 'positron' {
540540

541541
/** The URI of the notebook document associated with the session, if any */
542542
readonly notebookUri?: vscode.Uri;
543+
544+
/** The starting working directory of the session, if any */
545+
readonly workingDirectory?: string;
543546
}
544547

545548
/**

‎src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ import { IModelService } from '../../../../editor/common/services/model.js';
1515
import { ILanguageSelection, ILanguageService } from '../../../../editor/common/languages/language.js';
1616
import { ITextModelContentProvider, ITextModelService } from '../../../../editor/common/services/resolverService.js';
1717
import * as nls from '../../../../nls.js';
18+
// --- Start Positron ---
19+
// eslint-disable-next-line no-duplicate-imports
20+
import { ConfigurationScope } from '../../../../platform/configuration/common/configurationRegistry.js';
21+
// --- End Positron ---
1822
import { Extensions, IConfigurationPropertySchema, IConfigurationRegistry } from '../../../../platform/configuration/common/configurationRegistry.js';
1923
import { SyncDescriptor } from '../../../../platform/instantiation/common/descriptors.js';
2024
import { InstantiationType, registerSingleton } from '../../../../platform/instantiation/common/extensions.js';
@@ -1311,6 +1315,12 @@ configurationRegistry.registerConfiguration({
13111315
type: 'string',
13121316
default: '',
13131317
tags: ['notebookLayout']
1318+
},
1319+
[NotebookSetting.workingDirectory]: {
1320+
markdownDescription: nls.localize('notebook.workingDirectory', "Default working directory for notebook kernels. Supports variables like `${workspaceFolder}`. When empty, uses the notebook file's directory. Any change to this setting will apply to future opened notebooks."),
1321+
type: 'string',
1322+
default: '',
1323+
scope: ConfigurationScope.RESOURCE
13141324
}
13151325
}
13161326
});

‎src/vs/workbench/contrib/notebook/common/notebookCommon.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,6 +1065,7 @@ export const NotebookSetting = {
10651065
outputBackupSizeLimit: 'notebook.backup.sizeLimit',
10661066
multiCursor: 'notebook.multiCursor.enabled',
10671067
markupFontFamily: 'notebook.markup.fontFamily',
1068+
workingDirectory: 'notebook.workingDirectory',
10681069
} as const;
10691070

10701071
export const enum CellStatusbarAlignment {

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

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ import { INotificationService, Severity } from '../../../../platform/notificatio
2727
import { localize } from '../../../../nls.js';
2828
import { UiClientInstance } from '../../languageRuntime/common/languageRuntimeUiClient.js';
2929
import { IWorkbenchEnvironmentService } from '../../environment/common/environmentService.js';
30+
import { IConfigurationResolverService } from '../../configurationResolver/common/configurationResolver.js';
31+
import { IWorkspaceContextService } from '../../../../platform/workspace/common/workspace.js';
32+
import { NotebookSetting } from '../../../contrib/notebook/common/notebookCommon.js';
3033

3134
/**
3235
* The maximum number of active sessions a user can have running at a time.
@@ -170,7 +173,9 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
170173
@IExtensionService private readonly _extensionService: IExtensionService,
171174
@IStorageService private readonly _storageService: IStorageService,
172175
@IUpdateService private readonly _updateService: IUpdateService,
173-
@IWorkbenchEnvironmentService private readonly _environmentService: IWorkbenchEnvironmentService
176+
@IWorkbenchEnvironmentService private readonly _environmentService: IWorkbenchEnvironmentService,
177+
@IConfigurationResolverService private readonly _configurationResolverService: IConfigurationResolverService,
178+
@IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService
174179
) {
175180

176181
super();
@@ -387,6 +392,44 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
387392
return Array.from(this._activeSessionsBySessionId.values());
388393
}
389394

395+
/**
396+
* Resolves the working directory configuration with variable substitution.
397+
*
398+
* @param notebookUri The URI of the notebook, if any, for resource-scoped configuration
399+
* @returns The resolved working directory or undefined if not configured
400+
*/
401+
private async resolveWorkingDirectory(notebookUri?: URI): Promise<string | undefined> {
402+
// Get the working directory configuration
403+
const configValue = this._configurationService.getValue<string>(
404+
NotebookSetting.workingDirectory,
405+
notebookUri ? { resource: notebookUri } : {}
406+
);
407+
408+
// If no configuration value is set, return undefined
409+
if (!configValue || configValue.trim() === '') {
410+
return undefined;
411+
}
412+
413+
// Get the workspace folder for variable resolution
414+
const workspaceFolder = notebookUri
415+
? this._workspaceContextService.getWorkspaceFolder(notebookUri)
416+
: this._workspaceContextService.getWorkspace().folders[0];
417+
418+
try {
419+
// Resolve variables in the configuration value
420+
const resolvedValue = await this._configurationResolverService.resolveAsync(
421+
workspaceFolder || undefined,
422+
configValue
423+
);
424+
425+
return resolvedValue;
426+
} catch (error) {
427+
// Log the error and return the original value as fallback
428+
this._logService.warn(`Failed to resolve working directory variables in '${configValue}':`, error);
429+
return configValue;
430+
}
431+
}
432+
390433
/**
391434
* Select a session for the provided runtime.
392435
*
@@ -1535,10 +1578,15 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
15351578
}
15361579

15371580
const sessionId = this.generateNewSessionId(runtimeMetadata, sessionMode === LanguageRuntimeSessionMode.Notebook);
1581+
1582+
// Resolve the working directory configuration
1583+
const workingDirectory = await this.resolveWorkingDirectory(notebookUri);
1584+
15381585
const sessionMetadata: IRuntimeSessionMetadata = {
15391586
sessionId,
15401587
sessionMode,
15411588
notebookUri,
1589+
workingDirectory,
15421590
createdTimestamp: Date.now(),
15431591
startReason: source
15441592
};
@@ -1645,7 +1693,7 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
16451693
* @param runtime The runtime to get the manager for.
16461694
* @returns The session manager that manages the runtime.
16471695
*
1648-
* Throws an errror if no session manager is found for the runtime.
1696+
* Throws an error if no session manager is found for the runtime.
16491697
*/
16501698
private async getManagerForRuntime(runtime: ILanguageRuntimeMetadata): Promise<ILanguageRuntimeSessionManager> {
16511699
// Look for the session manager that manages the runtime.

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,9 @@ export interface IRuntimeSessionMetadata {
8585
/** The notebook associated with the session, if any */
8686
readonly notebookUri: URI | undefined;
8787

88+
/** The starting working directory of the session, if any */
89+
readonly workingDirectory?: string;
90+
8891
/**
8992
* A timestamp (in milliseconds since the Epoch) representing the time at
9093
* which the runtime session was created.

‎src/vs/workbench/services/runtimeSession/test/common/runtimeSession.test.ts

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import { TestLanguageRuntimeSession, waitForRuntimeState } from './testLanguageR
1919
import { createRuntimeServices, createTestLanguageRuntimeMetadata, startTestLanguageRuntimeSession } from './testRuntimeSessionService.js';
2020
import { TestRuntimeSessionManager } from '../../../../test/common/positronWorkbenchTestServices.js';
2121
import { TestWorkspaceTrustManagementService } from '../../../../test/common/workbenchTestServices.js';
22+
import { IConfigurationResolverService } from '../../../configurationResolver/common/configurationResolver.js';
23+
import { NotebookSetting } from '../../../../contrib/notebook/common/notebookCommon.js';
2224

2325
type IStartSessionTask = (runtime: ILanguageRuntimeMetadata) => Promise<TestLanguageRuntimeSession>;
2426

@@ -31,6 +33,7 @@ suite('Positron - RuntimeSessionService', () => {
3133
let runtimeSessionService: IRuntimeSessionService;
3234
let configService: TestConfigurationService;
3335
let workspaceTrustManagementService: TestWorkspaceTrustManagementService;
36+
let configurationResolverService: IConfigurationResolverService;
3437
let manager: TestRuntimeSessionManager;
3538
let runtime: ILanguageRuntimeMetadata;
3639
let anotherRuntime: ILanguageRuntimeMetadata;
@@ -44,6 +47,7 @@ suite('Positron - RuntimeSessionService', () => {
4447
runtimeSessionService = instantiationService.get(IRuntimeSessionService);
4548
configService = instantiationService.get(IConfigurationService) as TestConfigurationService;
4649
workspaceTrustManagementService = instantiationService.get(IWorkspaceTrustManagementService) as TestWorkspaceTrustManagementService;
50+
configurationResolverService = instantiationService.get(IConfigurationResolverService);
4751
manager = TestRuntimeSessionManager.instance;
4852

4953
// Dispose all sessions on teardown.
@@ -1182,4 +1186,91 @@ suite('Positron - RuntimeSessionService', () => {
11821186
assert.strictEqual(session.dynState.sessionName, newName, 'Session name should be updated correctly');
11831187
assert.strictEqual(otherSession.dynState.sessionName, runtime.runtimeName, 'Other session name should remain unchanged');
11841188
});
1189+
1190+
suite('Working Directory Configuration', () => {
1191+
test('working directory is passed to session metadata when configured', async () => {
1192+
const workingDir = '/custom/working/directory';
1193+
configService.setUserConfiguration(NotebookSetting.workingDirectory, workingDir);
1194+
1195+
const session = await startConsole(runtime);
1196+
1197+
assert.strictEqual(session.metadata.workingDirectory, workingDir, 'Working directory should be set in session metadata');
1198+
});
1199+
1200+
test('working directory is undefined when not configured', async () => {
1201+
// Ensure no working directory is configured
1202+
configService.setUserConfiguration(NotebookSetting.workingDirectory, '');
1203+
1204+
const session = await startConsole(runtime);
1205+
1206+
assert.strictEqual(session.metadata.workingDirectory, undefined, 'Working directory should be undefined when not configured');
1207+
});
1208+
1209+
test('working directory is undefined when configuration is empty string', async () => {
1210+
configService.setUserConfiguration(NotebookSetting.workingDirectory, '');
1211+
1212+
const session = await startConsole(runtime);
1213+
1214+
assert.strictEqual(session.metadata.workingDirectory, undefined, 'Working directory should be undefined for empty string');
1215+
});
1216+
1217+
test('working directory is undefined when configuration is whitespace only', async () => {
1218+
configService.setUserConfiguration(NotebookSetting.workingDirectory, ' ');
1219+
1220+
const session = await startConsole(runtime);
1221+
1222+
assert.strictEqual(session.metadata.workingDirectory, undefined, 'Working directory should be undefined for whitespace only');
1223+
});
1224+
1225+
test('working directory supports variable resolution', async () => {
1226+
const workingDir = '/workspace/folder';
1227+
configService.setUserConfiguration(NotebookSetting.workingDirectory, workingDir);
1228+
1229+
// Create a mock that actually resolves variables
1230+
const mockConfigResolver = configurationResolverService as any;
1231+
mockConfigResolver.resolveAsync = sinon.stub().resolves('/resolved/workspace/folder');
1232+
1233+
const session = await startConsole(runtime);
1234+
1235+
assert.strictEqual(session.metadata.workingDirectory, '/resolved/workspace/folder', 'Working directory should be resolved');
1236+
sinon.assert.calledOnce(mockConfigResolver.resolveAsync);
1237+
});
1238+
1239+
test('working directory falls back to original value when resolution fails', async () => {
1240+
const workingDir = '/workspace/folder';
1241+
configService.setUserConfiguration(NotebookSetting.workingDirectory, workingDir);
1242+
1243+
// Create a mock that throws an error during resolution
1244+
const mockConfigResolver = configurationResolverService as any;
1245+
mockConfigResolver.resolveAsync = sinon.stub().rejects(new Error('Resolution failed'));
1246+
1247+
const session = await startConsole(runtime);
1248+
1249+
assert.strictEqual(session.metadata.workingDirectory, workingDir, 'Working directory should fall back to original value');
1250+
sinon.assert.calledOnce(mockConfigResolver.resolveAsync);
1251+
});
1252+
1253+
test('working directory is resource-scoped for notebook sessions', async () => {
1254+
const workingDir = '/notebook/specific/directory';
1255+
await configService.setUserConfiguration(NotebookSetting.workingDirectory, workingDir, notebookUri);
1256+
1257+
const session = await startNotebook(runtime);
1258+
1259+
assert.strictEqual(session.metadata.workingDirectory, workingDir, 'Working directory should be resource-scoped');
1260+
});
1261+
1262+
test('working directory differs between console and notebook sessions', async () => {
1263+
const consoleWorkingDir = '/console/directory';
1264+
const notebookWorkingDir = '/notebook/directory';
1265+
1266+
await configService.setUserConfiguration(NotebookSetting.workingDirectory, consoleWorkingDir);
1267+
await configService.setUserConfiguration(NotebookSetting.workingDirectory, notebookWorkingDir, notebookUri);
1268+
1269+
const consoleSession = await startConsole(runtime);
1270+
const notebookSession = await startNotebook(runtime);
1271+
1272+
assert.strictEqual(consoleSession.metadata.workingDirectory, consoleWorkingDir, 'Console session should use global configuration');
1273+
assert.strictEqual(notebookSession.metadata.workingDirectory, notebookWorkingDir, 'Notebook session should use resource-scoped configuration');
1274+
});
1275+
});
11851276
});

‎src/vs/workbench/services/runtimeSession/test/common/testRuntimeSessionService.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,12 @@ import { IPositronModalDialogsService } from '../../../positronModalDialogs/comm
2626
import { RuntimeSessionService } from '../../common/runtimeSession.js';
2727
import { IRuntimeSessionService, RuntimeStartMode } from '../../common/runtimeSessionService.js';
2828
import { TestLanguageRuntimeSession } from './testLanguageRuntimeSession.js';
29-
import { TestOpenerService, TestPositronModalDialogService, TestCommandService, TestRuntimeSessionManager } from '../../../../test/common/positronWorkbenchTestServices.js';
30-
import { TestExtensionService, TestStorageService, TestWorkspaceTrustManagementService } from '../../../../test/common/workbenchTestServices.js';
29+
import { TestOpenerService, TestPositronModalDialogService, TestCommandService, TestRuntimeSessionManager, TestConfigurationResolverService } from '../../../../test/common/positronWorkbenchTestServices.js';
30+
import { TestExtensionService, TestStorageService, TestWorkspaceTrustManagementService, TestContextService } from '../../../../test/common/workbenchTestServices.js';
3131
import { INotificationService } from '../../../../../platform/notification/common/notification.js';
3232
import { TestNotificationService } from '../../../../../platform/notification/test/common/testNotificationService.js';
33+
import { IConfigurationResolverService } from '../../../configurationResolver/common/configurationResolver.js';
34+
import { IWorkspaceContextService } from '../../../../../platform/workspace/common/workspace.js';
3335

3436
export function createRuntimeServices(
3537
instantiationService: TestInstantiationService,
@@ -42,6 +44,8 @@ export function createRuntimeServices(
4244
instantiationService.stub(ILogService, new NullLogService());
4345
instantiationService.stub(IWorkspaceTrustManagementService, disposables.add(new TestWorkspaceTrustManagementService()));
4446
instantiationService.stub(IConfigurationService, new TestConfigurationService());
47+
instantiationService.stub(IWorkspaceContextService, new TestContextService());
48+
instantiationService.stub(IConfigurationResolverService, new TestConfigurationResolverService());
4549
instantiationService.stub(ILanguageRuntimeService, disposables.add(instantiationService.createInstance(LanguageRuntimeService)));
4650
instantiationService.stub(IPositronModalDialogsService, new TestPositronModalDialogService());
4751
instantiationService.stub(ICommandService, new TestCommandService(instantiationService));

‎src/vs/workbench/test/common/positronWorkbenchTestServices.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,17 @@
55

66
import { Emitter, Event } from '../../../base/common/event.js';
77
import { IDisposable } from '../../../base/common/lifecycle.js';
8+
import { IProcessEnvironment } from '../../../base/common/platform.js';
89
import { URI } from '../../../base/common/uri.js';
910
import { ICommandService, ICommandEvent, CommandsRegistry } from '../../../platform/commands/common/commands.js';
1011
import { IContextKeyService } from '../../../platform/contextkey/common/contextkey.js';
1112
import { IInstantiationService } from '../../../platform/instantiation/common/instantiation.js';
1213
import { IOpenerService, IOpener, IValidator, IExternalUriResolver, IExternalOpener, OpenInternalOptions, OpenExternalOptions, ResolveExternalUriOptions, IResolvedExternalUri } from '../../../platform/opener/common/opener.js';
14+
import { IWorkspaceFolder, IWorkspaceFolderData } from '../../../platform/workspace/common/workspace.js';
1315
import { NotebookCellTextModel } from '../../contrib/notebook/common/model/notebookCellTextModel.js';
1416
import { INotebookTextModel } from '../../contrib/notebook/common/notebookCommon.js';
1517
import { ICellExecutionParticipant, IDidEndNotebookCellsExecutionEvent, IDidStartNotebookCellsExecutionEvent, INotebookExecutionService } from '../../contrib/notebook/common/notebookExecutionService.js';
18+
import { IConfigurationResolverService } from '../../services/configurationResolver/common/configurationResolver.js';
1619
import { ILanguageRuntimeMetadata } from '../../services/languageRuntime/common/languageRuntimeService.js';
1720
import { IPositronModalDialogsService, ShowConfirmationModalDialogOptions, IModalDialogPromptInstance } from '../../services/positronModalDialogs/common/positronModalDialogs.js';
1821
import { ILanguageRuntimeSessionManager, IRuntimeSessionMetadata, ILanguageRuntimeSession } from '../../services/runtimeSession/common/runtimeSessionService.js';
@@ -149,6 +152,38 @@ export class TestRuntimeSessionManager implements ILanguageRuntimeSessionManager
149152
}
150153
}
151154

155+
export class TestConfigurationResolverService implements IConfigurationResolverService {
156+
_serviceBrand: undefined;
157+
158+
get resolvableVariables(): ReadonlySet<string> {
159+
return new Set();
160+
}
161+
162+
resolveAny(_folder: IWorkspaceFolder | undefined, value: any): any {
163+
return value;
164+
}
165+
166+
async resolveAsync(_folder: IWorkspaceFolder | undefined, value: any): Promise<any> {
167+
return value;
168+
}
169+
170+
resolveWithInteraction(_folder: IWorkspaceFolder | undefined, config: any): Promise<any> {
171+
return Promise.resolve(config);
172+
}
173+
174+
resolveWithEnvironment(_environment: IProcessEnvironment, _folder: IWorkspaceFolderData | undefined, value: string): Promise<string> {
175+
return Promise.resolve(value);
176+
}
177+
178+
resolveWithInteractionReplace(_folder: IWorkspaceFolder | undefined, config: any): Promise<any> {
179+
return Promise.resolve(config);
180+
}
181+
182+
contributeVariable(_variable: string, _resolver: () => Promise<string | undefined>): void {
183+
// Mock implementation - does nothing
184+
}
185+
}
186+
152187
/**
153188
* Re-export the TestPositronConsoleService and TestPositronConsoleInstance.
154189
* This allows test files to import from positronWorkbenchTestServices.ts

0 commit comments

Comments
 (0)