Skip to content

Commit 513ae03

Browse files
authored
Fix errors arising from kernels running on a separate extension host from other extensions (#9221)
This change fixes some issues that arise when extensions that try to use R/Python sessions are running on a different extension host than the kernels themselves. This fundamentally does not work today because sessions live in the extension host, and many of their methods are accessible only from that extension specific host or the main thread. It is not possible for one extension host to talk directly to another extension host. Adding full support for this is more than I wanted to bite off, so this change is a bit of a stopgap. Here's how it works: - API methods no longer return full-fledged session objects. Instead they return a more basic `BaseLanguageRuntimeSession` object. - When an extension asks for a session object for a session that is not known to the current extension host, we generate a kind of proxy that lets us talk to the session via the main thread. - In some extensions, we really and truly need a full session object. In those cases, we play a little bit dirty: we use `package.json`'s `extensionDependencies` field to declare a dependency on the kernels and/or supervisor. When VS Code is deciding which extensions to run on which host, it ensures that extensions that depend on each other are on the same host -- so we're guaranteed that the sessions in question live on our own host. That allow us to safely cast the `BaseLanguageRuntimeSession` to a full `LanguageRuntimeSessiion`. This makes a for a lot of running around to get e.g. a code result: ```mermaid graph TD e1[Extension 1 - Consumer] -- execute code --> eh1 eh1[Ext Host 1] -- session proxy --> p[Positron] p -- execute code --> eh2[Ext Host 2] eh2 -- execute code --> e2[Extension 2 - Kernel] e2 -- result --> eh2 eh2 -- result --> p p -- result --> eh1 eh1 -- result --> e1 ``` But it's about the best we can do given the architecture today. Addresses #9166. ### Release Notes <!-- Optionally, replace `N/A` with text to be included in the next release notes. The `N/A` bullets are ignored. If you refer to one or more Positron issues, these issues are used to collect information about the feature or bugfix, such as the relevant language pack as determined by Github labels of type `lang: `. The note will automatically be tagged with the language. These notes are typically filled by the Positron team. If you are an external contributor, you may ignore this section. --> #### New Features - N/A #### Bug Fixes - Fix errors when kernels are run on a separate extension host (#9166) ### QA Notes We don't actually know how Julia's system got into a state where it decided to put the kernels on a different extension host. But you can make yours do the same thing by adding this to your user settings JSON: ```json "extensions.experimental.affinity": { "positron.positron-r": 1, "positron.positron-supervisor": 1 } ``` It is possible that there are other problems in this configuration that I didn't fix. If adding the above setting gives you any trouble, check and see if it also repros on main. If it does, please file a separate issue; we think this config may be more common in the future.
1 parent ccb047e commit 513ae03

File tree

22 files changed

+413
-209
lines changed

22 files changed

+413
-209
lines changed

extensions/ipynb/src/ipynbMain.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ export function activate(context: vscode.ExtensionContext, serializer: vscode.No
8787
// --- Start Positron ---
8888
context.subscriptions.push(vscode.commands.registerCommand('ipynb.newUntitledIpynb', async (languageId?: string) => {
8989
// Try to use Positron's foreground session's language, fall back to 'plaintext'.
90-
const language = languageId ?? (await positron.runtime.getForegroundSession())?.runtimeMetadata?.languageId ?? 'plaintext';
90+
const language = languageId ?? (await positron.runtime.getForegroundSession())?.runtimeMetadata.languageId ?? 'plaintext';
9191
const cell = new vscode.NotebookCellData(vscode.NotebookCellKind.Code, '', language);
9292
const data = new vscode.NotebookData([cell]);
9393
data.metadata = {

extensions/positron-assistant/package.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -590,5 +590,8 @@
590590
"@anthropic-ai/sdk": "^0.57.0",
591591
"@github/copilot-language-server": "^1.335.0",
592592
"vscode-languageclient": "^9.0.1"
593-
}
593+
},
594+
"extensionDependencies": [
595+
"positron.positron-supervisor"
596+
]
594597
}

extensions/positron-assistant/src/tools.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -315,13 +315,7 @@ export function registerAssistantTools(
315315
]);
316316
}
317317

318-
let session: positron.LanguageRuntimeSession | undefined;
319-
const sessions = await positron.runtime.getActiveSessions();
320-
if (sessions && sessions.length > 0) {
321-
session = sessions.find(
322-
(session) => session.metadata.sessionId === options.input.sessionIdentifier,
323-
);
324-
}
318+
const session = await positron.runtime.getSession(options.input.sessionIdentifier);
325319
if (!session) {
326320
return new vscode.LanguageModelToolResult([
327321
new vscode.LanguageModelTextPart('[[]]')

extensions/positron-connections/package.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,9 @@
141141
]
142142
}
143143
},
144+
"extensionDependencies": [
145+
"positron.positron-supervisor"
146+
],
144147
"devDependencies": {
145148
"@types/glob": "^7.2.0",
146149
"@types/mocha": "^9.1.0",

extensions/positron-javascript/src/session.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,10 @@ export class JavaScriptLanguageRuntimeSession implements positron.LanguageRuntim
111111
return Promise.resolve(positron.RuntimeCodeFragmentStatus.Complete);
112112
}
113113

114+
getDynState(): Thenable<positron.LanguageRuntimeDynState> {
115+
return Promise.resolve(this.dynState);
116+
}
117+
114118
createClient(id: string, type: positron.RuntimeClientType, params: any): Thenable<void> {
115119
if (type === positron.RuntimeClientType.Variables) {
116120
// The only client type we support right now is an environment.

extensions/positron-python/src/client/llm-tools/llm-tools.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,9 @@ export function registerPythonLanguageModelTools(
1818
'getAttachedPythonPackages',
1919
{
2020
invoke: async (options, _token) => {
21-
let session: positron.LanguageRuntimeSession | undefined;
21+
let session: positron.BaseLanguageRuntimeSession | undefined;
2222
if (options.input.sessionIdentifier) {
23-
const sessions = await positron.runtime.getActiveSessions();
24-
if (sessions && sessions.length > 0) {
25-
session = sessions.find(
26-
(session) => session.metadata.sessionId === options.input.sessionIdentifier,
27-
);
28-
}
29-
23+
session = await positron.runtime.getSession(options.input.sessionIdentifier);
3024
if (!session) {
3125
session = await positron.runtime.getForegroundSession();
3226
}

extensions/positron-python/src/client/positron/manager.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,9 @@ export class PythonRuntimeManager implements IPythonRuntimeManager, Disposable {
435435
if (sessionsToShutdown.length > 0) {
436436
traceInfo(`Shutting down ${sessionsToShutdown.length} sessions using Python runtime at ${pythonPath}`);
437437
await Promise.all(
438-
sessionsToShutdown.map((session) => session.shutdown(positron.RuntimeExitReason.Shutdown)),
438+
sessionsToShutdown.map(async (session) => {
439+
session.shutdown(positron.RuntimeExitReason.Shutdown);
440+
}),
439441
);
440442
// Remove the runtime from our registry so we can recreate it
441443
this.registeredPythonRuntimes.delete(pythonPath);

extensions/positron-python/src/client/positron/session.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ export class PythonRuntimeSession implements positron.LanguageRuntimeSession, vs
118118
/** Information about the runtime that is only available after starting */
119119
private _runtimeInfo?: positron.LanguageRuntimeInfo;
120120

121-
dynState: positron.LanguageRuntimeDynState;
121+
private dynState: positron.LanguageRuntimeDynState;
122122

123123
onDidReceiveRuntimeMessage = this._messageEmitter.event;
124124

@@ -168,6 +168,10 @@ export class PythonRuntimeSession implements positron.LanguageRuntimeSession, vs
168168
return this._runtimeInfo;
169169
}
170170

171+
getDynState(): Thenable<positron.LanguageRuntimeDynState> {
172+
return Promise.resolve(this.dynState);
173+
}
174+
171175
async debug(request: positron.DebugProtocolRequest): Promise<positron.DebugProtocolResponse> {
172176
if (this._kernel) {
173177
return await this._kernel.debug(request);

extensions/positron-r/src/session.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,10 @@ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposa
148148
return this._kernel?.runtimeInfo;
149149
}
150150

151+
getDynState(): Thenable<positron.LanguageRuntimeDynState> {
152+
return Promise.resolve(this.dynState);
153+
}
154+
151155
/**
152156
* Opens a resource in the runtime.
153157
* @param resource The resource to open.

extensions/positron-reticulate/src/extension.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,13 @@ export class ReticulateRuntimeManager implements positron.LanguageRuntimeManager
179179
// We have a free R session, we can attach to it. First we need to figure out if there's
180180
// a prefered one.
181181
// TODO: maybe show a quick menu so the user can select the session they want to attach to?
182-
return freeRSessions[0];
182+
const session = freeRSessions[0];
183+
184+
// Resolve the session to a full session object. Note that this
185+
// typecast is only safe because we ensure that this extension
186+
// runs on the same extension host as the Python and R
187+
// extensions, via declared dependencies in package.json.
188+
return session as positron.LanguageRuntimeSession;
183189
} else {
184190
progress.report({ increment: 2, message: vscode.l10n.t('Starting a new R session') });
185191
// We need to create a new R session.
@@ -297,8 +303,10 @@ export class ReticulateRuntimeManager implements positron.LanguageRuntimeManager
297303
progress.report({ increment: 10, message: vscode.l10n.t('Finding the host R session') });
298304
const rSession = await (async () => {
299305
for (let attempt = 1; attempt <= 5; attempt++) {
300-
const sessions = await positron.runtime.getActiveSessions();
301-
const hostRSession = sessions.find((sess) => sess.metadata.sessionId === hostRSessionId);
306+
// Note that this typecast is only safe because we ensure that
307+
// this extension runs on the same extension host as the Python
308+
// and R extensions, via declared dependencies in package.json.
309+
const hostRSession = await positron.runtime.getSession(hostRSessionId) as positron.LanguageRuntimeSession | undefined;
302310
if (hostRSession) {
303311
return hostRSession;
304312
}
@@ -652,6 +660,10 @@ class ReticulateRuntimeSession implements positron.LanguageRuntimeSession {
652660
);
653661
}
654662

663+
getDynState(): Thenable<positron.LanguageRuntimeDynState> {
664+
return this.pythonSession.getDynState();
665+
}
666+
655667
createPythonRuntimeSession(runtimeMetadata: positron.LanguageRuntimeMetadata, sessionMetadata: positron.RuntimeSessionMetadata, kernelSpec?: JupyterKernelSpec): positron.LanguageRuntimeSession {
656668
const api = vscode.extensions.getExtension('ms-python.python');
657669
if (!api) {
@@ -746,10 +758,6 @@ class ReticulateRuntimeSession implements positron.LanguageRuntimeSession {
746758
return this.pythonSession.runtimeMetadata;
747759
}
748760

749-
public get dynState() {
750-
return this.pythonSession.dynState;
751-
}
752-
753761
public get runtimeInfo() {
754762
return this.pythonSession.runtimeInfo;
755763
}

0 commit comments

Comments
 (0)