Skip to content

Commit fb3b7c2

Browse files
committed
wip
1 parent 466b7f2 commit fb3b7c2

File tree

9 files changed

+157
-30
lines changed

9 files changed

+157
-30
lines changed

extensions/positron-supervisor/src/KallichoreSession.ts

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,11 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession {
158158
*/
159159
private _activeSession: ActiveSession | undefined;
160160

161+
/**
162+
* The working directory provided when starting the session, if any.
163+
*/
164+
private _providedWorkingDirectory: string | undefined;
165+
161166
/**
162167
* The message header for the current requests if any is active. This is
163168
* used for input requests (e.g. from `readline()` in R) Concurrent requests
@@ -340,18 +345,23 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession {
340345
this._kernelSpec = kernelSpec;
341346
const varActions = await this.buildEnvVarActions(false);
342347

343-
// Prepare the working directory; use the workspace root if available,
344-
// otherwise the home directory
345-
let workingDir = vscode.workspace.workspaceFolders?.[0].uri.fsPath || os.homedir();
346-
347-
// If we have a notebook URI, use its parent directory as the working
348-
// directory instead. Note that not all notebooks have valid on-disk
349-
// URIs since they may be transient or not yet saved; for these, we fall
350-
// back to the workspace root or home directory.
351-
if (this.metadata.notebookUri?.fsPath) {
352-
const notebookPath = this.metadata.notebookUri.fsPath;
353-
if (fs.existsSync(notebookPath)) {
354-
workingDir = path.dirname(notebookPath);
348+
// Prepare the working directory; use the configured working directory if provided,
349+
// otherwise use the workspace root if available, otherwise the home directory
350+
let workingDir = this._providedWorkingDirectory;
351+
352+
if (!workingDir) {
353+
// Default to workspace root or home directory
354+
workingDir = vscode.workspace.workspaceFolders?.[0].uri.fsPath || os.homedir();
355+
356+
// If we have a notebook URI, use its parent directory as the working
357+
// directory instead. Note that not all notebooks have valid on-disk
358+
// URIs since they may be transient or not yet saved; for these, we fall
359+
// back to the workspace root or home directory.
360+
if (this.metadata.notebookUri?.fsPath) {
361+
const notebookPath = this.metadata.notebookUri.fsPath;
362+
if (fs.existsSync(notebookPath)) {
363+
workingDir = path.dirname(notebookPath);
364+
}
355365
}
356366
}
357367

@@ -1034,7 +1044,10 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession {
10341044
*
10351045
* @returns The kernel info for the session.
10361046
*/
1037-
async start(): Promise<positron.LanguageRuntimeInfo> {
1047+
async start(workingDirectory?: string): Promise<positron.LanguageRuntimeInfo> {
1048+
// Store the provided working directory for use during session creation
1049+
this._providedWorkingDirectory = workingDirectory;
1050+
10381051
// If this session needs to be started by an external provider, do that
10391052
// instead of asking the supervisor to start it.
10401053
if (this._kernelSpec?.startKernel) {

src/positron-dts/positron.d.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1064,8 +1064,9 @@ declare module 'positron' {
10641064
* If the runtime fails to start for any reason, the Thenable should reject with an error
10651065
* object containing a `message` field with a human-readable error message and an optional
10661066
* `details` field with additional information.
1067+
* @param workingDirectory Optional working directory for the runtime session
10671068
*/
1068-
start(): Thenable<LanguageRuntimeInfo>;
1069+
start(workingDirectory?: string): Thenable<LanguageRuntimeInfo>;
10691070

10701071
/**
10711072
* Interrupt the runtime; returns a Thenable that resolves when the interrupt has been

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."),
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/positronHistory/test/common/executionHistoryService.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { IRuntimeAutoStartEvent, IRuntimeStartupService, ISessionRestoreFailedEv
1515
import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
1616
import { ExecutionHistoryService } from '../../common/executionHistory.js';
1717
import { IWorkspace, IWorkspaceContextService, IWorkspaceFoldersWillChangeEvent } from '../../../../../platform/workspace/common/workspace.js';
18-
import { ILanguageRuntimeExit, ILanguageRuntimeMetadata, ILanguageRuntimeSessionState, IRuntimeManager, LanguageRuntimeSessionLocation, LanguageRuntimeSessionMode, LanguageRuntimeStartupBehavior, RuntimeExitReason, RuntimeState } from '../../../../services/languageRuntime/common/languageRuntimeService.js';
18+
import { ILanguageRuntimeExit, ILanguageRuntimeInfo, ILanguageRuntimeMetadata, ILanguageRuntimeSessionState, IRuntimeManager, LanguageRuntimeSessionLocation, LanguageRuntimeSessionMode, LanguageRuntimeStartupBehavior, RuntimeExitReason, RuntimeState } from '../../../../services/languageRuntime/common/languageRuntimeService.js';
1919
import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js';
2020
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
2121
import { Emitter } from '../../../../../base/common/event.js';
@@ -519,7 +519,7 @@ class TestLanguageRuntimeSession extends Disposable implements ILanguageRuntimeS
519519
throw new Error('Method not implemented.');
520520
}
521521

522-
start(_showBanner?: boolean): Promise<any> {
522+
start(_workingDirectory?: string): Promise<ILanguageRuntimeInfo> {
523523
throw new Error('Method not implemented.');
524524
}
525525

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

Lines changed: 64 additions & 11 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
*
@@ -466,6 +509,9 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
466509
}
467510
}
468511

512+
// Resolve the working directory configuration
513+
const workingDirectory = await this.resolveWorkingDirectory(notebookUri);
514+
469515
// Wait for the selected runtime to start.
470516
await this.startNewRuntimeSession(
471517
runtime.runtimeId,
@@ -474,7 +520,8 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
474520
notebookUri,
475521
source,
476522
startMode,
477-
true
523+
true,
524+
workingDirectory
478525
);
479526
}
480527

@@ -567,6 +614,7 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
567614
* @param source The source of the request to start the runtime.
568615
* @param startMode The mode in which to start the runtime.
569616
* @param activate Whether to activate/focus the session after it is started.
617+
* @param workingDirectory The working directory to use for the session, if any.
570618
*/
571619
async startNewRuntimeSession(
572620
runtimeId: string,
@@ -575,7 +623,8 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
575623
notebookUri: URI | undefined,
576624
source: string,
577625
startMode = RuntimeStartMode.Starting,
578-
activate: boolean): Promise<string> {
626+
activate: boolean,
627+
workingDirectory?: string): Promise<string> {
579628
// See if we are already starting the requested session. If we
580629
// are, return the promise that resolves when the session is ready to
581630
// use. This makes it possible for multiple requests to start the same
@@ -611,7 +660,7 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
611660
this._logService.info(
612661
`Starting session for language runtime ` +
613662
`${formatLanguageRuntimeMetadata(languageRuntime)} (Source: ${source})`);
614-
return this.doCreateRuntimeSession(languageRuntime, sessionName, sessionMode, source, startMode, activate, notebookUri);
663+
return this.doCreateRuntimeSession(languageRuntime, sessionName, sessionMode, source, startMode, activate, notebookUri, workingDirectory);
615664
}
616665

617666
/**
@@ -742,7 +791,7 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
742791
}
743792

744793
// Actually reconnect the session.
745-
await this.doStartRuntimeSession(session, sessionManager, RuntimeStartMode.Reconnecting, activate);
794+
await this.doStartRuntimeSession(session, sessionManager, RuntimeStartMode.Reconnecting, activate, undefined);
746795

747796
return sessionMetadata.sessionId;
748797

@@ -1061,7 +1110,7 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
10611110
// Restart the working directory in the same directory as the session.
10621111
if (session.getRuntimeState() === RuntimeState.Exited) {
10631112
// Do a start, behind the scenes
1064-
await session.start();
1113+
await session.start(activeSession.workingDirectory);
10651114
} else {
10661115
await session.restart(activeSession.workingDirectory);
10671116
}
@@ -1486,7 +1535,7 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
14861535
}
14871536
}
14881537

1489-
return this.doCreateRuntimeSession(metadata, metadata.runtimeName, sessionMode, source, RuntimeStartMode.Starting, activate, notebookUri);
1538+
return this.doCreateRuntimeSession(metadata, metadata.runtimeName, sessionMode, source, RuntimeStartMode.Starting, activate, notebookUri, undefined);
14901539
}
14911540

14921541
/**
@@ -1499,6 +1548,7 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
14991548
* @param startMode The mode in which to start the runtime.
15001549
* @param activate Whether to activate/focus the session after it is started.
15011550
* @param notebookDocument The notebook document to attach to the session, if any.
1551+
* @param workingDirectory The working directory to use for the session, if any.
15021552
*
15031553
* Returns a promise that resolves with the session ID when the runtime is
15041554
* ready to use.
@@ -1509,7 +1559,8 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
15091559
source: string,
15101560
startMode: RuntimeStartMode,
15111561
activate: boolean,
1512-
notebookUri?: URI): Promise<string> {
1562+
notebookUri?: URI,
1563+
workingDirectory?: string): Promise<string> {
15131564
this.setStartingSessionMaps(sessionMode, runtimeMetadata, notebookUri);
15141565

15151566
// Create a promise that resolves when the runtime is ready to use, if there isn't already one.
@@ -1560,7 +1611,7 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
15601611

15611612
// Actually start the session.
15621613
try {
1563-
await this.doStartRuntimeSession(session, sessionManager, startMode, activate);
1614+
await this.doStartRuntimeSession(session, sessionManager, startMode, activate, workingDirectory);
15641615
startPromise.complete(sessionId);
15651616
} catch (err) {
15661617
startPromise.error(err);
@@ -1576,11 +1627,13 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
15761627
* @param manager The session manager for the session.
15771628
* @param startMode The mode in which the session is starting.
15781629
* @param activate Whether to activate/focus the session after it is started.
1630+
* @param workingDirectory The working directory to use for the session, if any.
15791631
*/
15801632
private async doStartRuntimeSession(session: ILanguageRuntimeSession,
15811633
manager: ILanguageRuntimeSessionManager,
15821634
startMode: RuntimeStartMode,
1583-
activate: boolean):
1635+
activate: boolean,
1636+
workingDirectory?: string):
15841637
Promise<void> {
15851638
// Fire the onWillStartRuntime event.
15861639
const evt: IRuntimeSessionWillStartEvent = {
@@ -1595,7 +1648,7 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
15951648

15961649
try {
15971650
// Attempt to start, or reconnect to, the session.
1598-
await session.start();
1651+
await session.start(workingDirectory);
15991652

16001653
// The session has started. Move it from the starting runtimes to the
16011654
// running runtimes.

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ export interface ILanguageRuntimeSession extends IDisposable {
202202
*/
203203
setWorkingDirectory(directory: string): Thenable<void>;
204204

205-
start(): Thenable<ILanguageRuntimeInfo>;
205+
start(workingDirectory?: string): Thenable<ILanguageRuntimeInfo>;
206206

207207
/** Interrupt the runtime */
208208
interrupt(): void;
@@ -408,6 +408,7 @@ export interface IRuntimeSessionService {
408408
* @param startMode The mode in which to start the runtime.
409409
* @param activate Whether to activate/focus the session after it is
410410
* started.
411+
* @param workingDirectory Optional working directory for the runtime session.
411412
*
412413
* Returns a promise that resolves to the session ID of the new session.
413414
*/
@@ -418,7 +419,8 @@ export interface IRuntimeSessionService {
418419
notebookUri: URI | undefined,
419420
source: string,
420421
startMode: RuntimeStartMode,
421-
activate: boolean): Promise<string>;
422+
activate: boolean,
423+
workingDirectory?: string): Promise<string>;
422424

423425
/**
424426
* Validates a persisted runtime session before reconnecting to it.

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

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ 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 { NotebookSetting } from '../../../../../workbench/contrib/notebook/common/notebookCommon.js';
2223

2324
type IStartSessionTask = (runtime: ILanguageRuntimeMetadata) => Promise<TestLanguageRuntimeSession>;
2425

@@ -1182,4 +1183,45 @@ suite('Positron - RuntimeSessionService', () => {
11821183
assert.strictEqual(session.dynState.sessionName, newName, 'Session name should be updated correctly');
11831184
assert.strictEqual(otherSession.dynState.sessionName, runtime.runtimeName, 'Other session name should remain unchanged');
11841185
});
1186+
1187+
test('working directory configuration is passed to session start', async () => {
1188+
// Set working directory configuration
1189+
const testWorkingDir = '/test/working/directory';
1190+
configService.setUserConfiguration(NotebookSetting.workingDirectory, testWorkingDir);
1191+
1192+
// Create session and verify working directory is passed
1193+
const session = await startConsole(runtime);
1194+
await waitForRuntimeState(session, RuntimeState.Ready);
1195+
1196+
// Verify that the session received the working directory
1197+
const startCall = sinon.spy(session, 'start');
1198+
// For this test, we need to restart to verify the parameter passing
1199+
await runtimeSessionService.restartSession(session.sessionId, 'test');
1200+
await waitForRuntimeState(session, RuntimeState.Ready);
1201+
1202+
// Check that start was called with the working directory
1203+
sinon.assert.calledWith(startCall, testWorkingDir);
1204+
});
1205+
1206+
test('working directory configuration is resource-scoped for notebooks', async () => {
1207+
const testWorkingDir = '/test/notebook/directory';
1208+
const notebookUri = URI.file('/path/to/test.ipynb');
1209+
1210+
// Set resource-scoped working directory configuration
1211+
configService.setUserConfiguration(NotebookSetting.workingDirectory, testWorkingDir, notebookUri);
1212+
1213+
// Start a notebook session
1214+
await runtimeSessionService.selectRuntime(runtime.runtimeId, 'test', notebookUri);
1215+
const session = runtimeSessionService.getNotebookSessionForNotebookUri(notebookUri);
1216+
assert.ok(session, 'Notebook session should be created');
1217+
1218+
await waitForRuntimeState(session, RuntimeState.Ready);
1219+
1220+
// Verify working directory was applied (by checking that the start method was called correctly)
1221+
const startSpy = sinon.spy(session, 'start');
1222+
await runtimeSessionService.restartSession(session.sessionId, 'test');
1223+
await waitForRuntimeState(session, RuntimeState.Ready);
1224+
1225+
sinon.assert.calledWith(startSpy, testWorkingDir);
1226+
});
11851227
});

0 commit comments

Comments
 (0)