From a4d365dbeb030a6799a5be824a7090663df347c2 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 12 Jun 2025 11:30:33 +0200 Subject: [PATCH 01/48] Implement async channel --- extensions/positron-supervisor/src/Channel.ts | 79 +++++++++++++++++++ extensions/positron-supervisor/src/util.ts | 4 + 2 files changed, 83 insertions(+) create mode 100644 extensions/positron-supervisor/src/Channel.ts diff --git a/extensions/positron-supervisor/src/Channel.ts b/extensions/positron-supervisor/src/Channel.ts new file mode 100644 index 000000000000..bb8daa79cd14 --- /dev/null +++ b/extensions/positron-supervisor/src/Channel.ts @@ -0,0 +1,79 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2025 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as vscode from 'vscode'; +import { delay } from './util'; + +// Used to yield periodically to the event loop +const YIELD_THRESHOLD = 100; + +/** + * Multi-consumer multi-producer channel. + * - Used as an async iterator. Dispose to close. + * - All endpoints can close. All are closed at the same time. + * - Sending a value to a closed channel is an error. + */ +export class Channel implements AsyncIterable, AsyncIterator, vscode.Disposable { + private closed = false; + private queue: T[] = []; + private pending_consumers: ((value: IteratorResult) => void)[] = []; + private i = 0; + + send(value: T) { + if (this.closed) { + throw new Error('Can\'t send values after channel is closed'); + } + + if (this.pending_consumers.length > 0) { + // There is a consumer waiting, resolve it immediately + this.pending_consumers.shift()!({ value, done: false }); + } else { + // No consumer waiting, queue up the value + this.queue.push(value); + } + } + + close() { + this.closed = true; + + // Resolve all pending consumers as done + while (this.pending_consumers.length > 0) { + this.pending_consumers.shift()!({ value: undefined, done: true }); + } + } + + dispose() { + this.close(); + } + + [Symbol.asyncIterator]() { + return this; + } + + async next(): Promise> { + if (this.queue.length > 0) { + ++this.i; + + // Yield regularly to event loop to avoid starvation. Sends are + // synchronous and handlers might be synchronous as well. + if (this.i > YIELD_THRESHOLD) { + this.i = 0; + await delay(0); + } + + return { value: this.queue.shift()!, done: false }; + } + + // If nothing in the queue and the channel is closed, we're done + if (this.closed) { + return { value: undefined, done: true }; + } + + // Nothing in the queue, wait for a value to be sent + return new Promise>((resolve) => { + this.pending_consumers.push(resolve); + }); + } +} diff --git a/extensions/positron-supervisor/src/util.ts b/extensions/positron-supervisor/src/util.ts index 880deed76dcd..c91dac7f68c8 100644 --- a/extensions/positron-supervisor/src/util.ts +++ b/extensions/positron-supervisor/src/util.ts @@ -251,3 +251,7 @@ export function unpackSerializedObjectWithBuffers(payload: unknown): { export function isEnumMember>(value: unknown, enumObj: T): value is T[keyof T] { return Object.values(enumObj).includes(value as T[keyof T]); } + +export function delay(ms: number) { + return new Promise(resolve => setTimeout(resolve, ms)); +} From 1ae57db3acff6ed0fef98f963fe94a62a9267bef Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 12 Jun 2025 12:18:22 +0200 Subject: [PATCH 02/48] Draft unmanaged comms --- .../src/KallichoreSession.ts | 60 ++++++++++++++++--- 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 716fba842bce..2c4edddb3019 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -41,6 +41,8 @@ import { createUniqueId, summarizeError, summarizeHttpError } from './util'; import { AdoptedSession } from './AdoptedSession'; import { DebugRequest } from './jupyter/DebugRequest'; import { JupyterMessageType } from './jupyter/JupyterMessageType.js'; +import { Channel } from './Channel'; +import { JupyterCommClose } from './jupyter/JupyterCommClose'; /** * The reason for a disconnection event. @@ -147,8 +149,11 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { */ private _profileChannel: vscode.OutputChannel | undefined; - /** A map of active comm channels */ - private readonly _comms: Map = new Map(); + /** A map of active comms connected to Positron clients */ + private readonly _clients: Map = new Map(); + + /** A map of active comms unmanaged by Positron */ + private readonly _comms: Map>> = new Map(); /** The kernel's log file, if any. */ private _kernelLogFile: string | undefined; @@ -564,7 +569,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { const request = new UICommRequest(method, args, promise); // Find the UI comm - const uiComm = Array.from(this._comms.values()) + const uiComm = Array.from(this._clients.values()) .find(c => c.target === positron.RuntimeClientType.Ui); if (!uiComm) { @@ -736,6 +741,23 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { } } + async createComm( + type: string, + params: Record, + ): Promise { + const id = `extension-comm-${type}-${this.runtimeMetadata.languageId}-${createUniqueId()}`; + + const msg: JupyterCommOpen = { + target_name: type, + comm_id: id, + data: params + }; + const commOpen = new CommOpenCommand(msg); + await this.sendCommand(commOpen); + + this._comms.set(id, new Channel()); + } + /** * Create a new client comm. * @@ -767,7 +789,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { }; const commOpen = new CommOpenCommand(msg, metadata); await this.sendCommand(commOpen); - this._comms.set(id, new Comm(id, type)); + this._clients.set(id, new Comm(id, type)); // If we have any pending UI comm requests and we just created the // UI comm, send them now @@ -798,8 +820,8 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { const target = comms[key].target_name; result[key] = target; // If we don't have a comm object for this comm, create one - if (!this._comms.has(key)) { - this._comms.set(key, new Comm(key, target)); + if (!this._clients.has(key)) { + this._clients.set(key, new Comm(key, target)); } // If we just discovered a UI comm, send any pending UI comm @@ -1752,7 +1774,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { } // All comms are now closed - this._comms.clear(); + this._clients.clear(); // Clear any starting comms this._startingComms.forEach((promise) => { @@ -1890,13 +1912,35 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { } } + // Handle comms that are not managed by Positron first + if (msg.header.msg_type === 'comm_close') { + const closeMsg = msg.content as JupyterCommClose; + const channel = this._comms.get(closeMsg.comm_id); + + if (channel) { + channel.close(); + this._comms.delete(closeMsg.comm_id); + return; + } + } + if (msg.header.msg_type === 'comm_msg') { + const commMsg = msg.content as JupyterCommMsg; + const channel = this._comms.get(commMsg.comm_id); + + if (channel) { + channel.send(commMsg.data); + return; + } + } + + // TODO: Make DAP and LSP comms unmanaged if (msg.header.msg_type === 'comm_msg') { const commMsg = msg.content as JupyterCommMsg; // If we have a DAP client active and this is a comm message intended // for that client, forward the message. if (this._dapClient) { - const comm = this._comms.get(commMsg.comm_id); + const comm = this._clients.get(commMsg.comm_id); if (comm && comm.id === this._dapClient.clientId) { this._dapClient.handleDapMessage(commMsg.data); } From 3d386bc8b1a34209f1c8656462c4d5822b828c8c Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 12 Jun 2025 12:58:48 +0200 Subject: [PATCH 03/48] Clean up comm channel when disposed --- extensions/positron-supervisor/src/Channel.ts | 18 ++++++++++++- .../src/KallichoreSession.ts | 26 ++++++++++++++++--- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/extensions/positron-supervisor/src/Channel.ts b/extensions/positron-supervisor/src/Channel.ts index bb8daa79cd14..9f809323cd84 100644 --- a/extensions/positron-supervisor/src/Channel.ts +++ b/extensions/positron-supervisor/src/Channel.ts @@ -20,6 +20,8 @@ export class Channel implements AsyncIterable, AsyncIterator, vscode.Di private queue: T[] = []; private pending_consumers: ((value: IteratorResult) => void)[] = []; private i = 0; + private disposables: vscode.Disposable[] = []; + private isDisposed = false; send(value: T) { if (this.closed) { @@ -35,7 +37,7 @@ export class Channel implements AsyncIterable, AsyncIterator, vscode.Di } } - close() { + private close() { this.closed = true; // Resolve all pending consumers as done @@ -45,9 +47,23 @@ export class Channel implements AsyncIterable, AsyncIterator, vscode.Di } dispose() { + // Since channel is owned by multiple endpoints we need to be careful about + // `dispose()` being idempotent + if (this.isDisposed) { + return; + } + this.isDisposed = true; + this.close(); + for (const disposable of this.disposables) { + disposable.dispose(); + } } + register(disposable: vscode.Disposable) { + this.disposables.push(disposable) + } + [Symbol.asyncIterator]() { return this; } diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 2c4edddb3019..19b713c647d9 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -73,6 +73,8 @@ export interface DisconnectedEvent { reason: DisconnectReason; } +export type CommChannel = Channel>; + export class KallichoreSession implements JupyterLanguageRuntimeSession { /** * The runtime messages emitter; consumes Jupyter messages and translates @@ -153,7 +155,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { private readonly _clients: Map = new Map(); /** A map of active comms unmanaged by Positron */ - private readonly _comms: Map>> = new Map(); + private readonly _comms: Map = new Map(); /** The kernel's log file, if any. */ private _kernelLogFile: string | undefined; @@ -744,7 +746,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { async createComm( type: string, params: Record, - ): Promise { + ): Promise { const id = `extension-comm-${type}-${this.runtimeMetadata.languageId}-${createUniqueId()}`; const msg: JupyterCommOpen = { @@ -755,7 +757,21 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { const commOpen = new CommOpenCommand(msg); await this.sendCommand(commOpen); - this._comms.set(id, new Channel()); + const channel: CommChannel = new Channel(); + + this._comms.set(id, channel); + channel.register({ + dispose: () => { + // If already deleted, it means a `comm_close` from the backend was + // received and we don't need to send one. + if (this._comms.delete(id)) { + const commClose = new CommCloseCommand(id); + this.sendCommand(commClose); + } + } + }); + + return channel; } /** @@ -1918,8 +1934,10 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { const channel = this._comms.get(closeMsg.comm_id); if (channel) { - channel.close(); + // Delete first, this prevents the channel disposable from sending a + // `comm_close` back this._comms.delete(closeMsg.comm_id); + channel.dispose(); return; } } From 6a962cd543d6a19ffc7bf93e0fdece61e5f1fa06 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 12 Jun 2025 17:23:43 +0200 Subject: [PATCH 04/48] Make raw comm bidirectional --- extensions/positron-supervisor/src/Channel.ts | 2 +- .../src/KallichoreSession.ts | 123 +++++++++++++++--- extensions/positron-supervisor/src/RawComm.ts | 44 +++++++ 3 files changed, 152 insertions(+), 17 deletions(-) create mode 100644 extensions/positron-supervisor/src/RawComm.ts diff --git a/extensions/positron-supervisor/src/Channel.ts b/extensions/positron-supervisor/src/Channel.ts index 9f809323cd84..eb0072b7a372 100644 --- a/extensions/positron-supervisor/src/Channel.ts +++ b/extensions/positron-supervisor/src/Channel.ts @@ -61,7 +61,7 @@ export class Channel implements AsyncIterable, AsyncIterator, vscode.Di } register(disposable: vscode.Disposable) { - this.disposables.push(disposable) + this.disposables.push(disposable); } [Symbol.asyncIterator]() { diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 19b713c647d9..84e173f484fa 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -43,6 +43,7 @@ import { DebugRequest } from './jupyter/DebugRequest'; import { JupyterMessageType } from './jupyter/JupyterMessageType.js'; import { Channel } from './Channel'; import { JupyterCommClose } from './jupyter/JupyterCommClose'; +import { CommBackendChannel, CommRpcMessage, CommRpcResponse, RawComm } from './RawComm'; /** * The reason for a disconnection event. @@ -73,8 +74,6 @@ export interface DisconnectedEvent { reason: DisconnectReason; } -export type CommChannel = Channel>; - export class KallichoreSession implements JupyterLanguageRuntimeSession { /** * The runtime messages emitter; consumes Jupyter messages and translates @@ -155,7 +154,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { private readonly _clients: Map = new Map(); /** A map of active comms unmanaged by Positron */ - private readonly _comms: Map = new Map(); + private readonly _comms: Map = new Map(); /** The kernel's log file, if any. */ private _kernelLogFile: string | undefined; @@ -746,19 +745,10 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { async createComm( type: string, params: Record, - ): Promise { + ): Promise { const id = `extension-comm-${type}-${this.runtimeMetadata.languageId}-${createUniqueId()}`; - const msg: JupyterCommOpen = { - target_name: type, - comm_id: id, - data: params - }; - const commOpen = new CommOpenCommand(msg); - await this.sendCommand(commOpen); - - const channel: CommChannel = new Channel(); - + const channel: CommBackendChannel = new Channel(); this._comms.set(id, channel); channel.register({ dispose: () => { @@ -771,7 +761,15 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { } }); - return channel; + const msg: JupyterCommOpen = { + target_name: type, + comm_id: id, + data: params + }; + const commOpen = new CommOpenCommand(msg); + await this.sendCommand(commOpen); + + return new RawCommImpl(id, this, channel); } /** @@ -1904,6 +1902,16 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { if (request.replyType === msg.header.msg_type) { request.resolve(msg.content); this._pendingRequests.delete(msg.parent_header.msg_id); + + // If this is a reply for an unmanaged comm, return early. + // The comm socket gets the response via the now resolved request + // promise. + if (msg.header.msg_type === 'comm_msg') { + const commMsg = msg.content as JupyterCommMsg; + if (this._comms.has(commMsg.comm_id)) { + return; + } + } } } } @@ -1946,7 +1954,13 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { const channel = this._comms.get(commMsg.comm_id); if (channel) { - channel.send(commMsg.data); + const rpcMsg = commMsg.data as CommRpcMessage; + if (rpcMsg.id) { + const req = new CommBackendRequest(this, commMsg.comm_id, rpcMsg); + channel.send(req); + } else { + channel.send({ kind: 'notification', method: rpcMsg.method, params: rpcMsg.params }); + } return; } } @@ -2131,3 +2145,80 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { } } } + +class RawCommImpl { + constructor( + private readonly commId: string, + private readonly session: KallichoreSession, + public readonly receiver: CommBackendChannel, + ) {} + + notify(method: string, params?: Record) { + const msg: CommRpcMessage = { + jsonrpc: '2.0', + method, + params, + }; + + // We don't expect a response here, so `id` can be created and forgotten + const id = createUniqueId(); + this.session.sendClientMessage(this.commId, id, msg); + } + + async request(method: string, params?: Record): Promise { + const id = createUniqueId(); + + const msg: CommRpcMessage = { + jsonrpc: '2.0', + id, + method, + params, + }; + + const commMsg: JupyterCommMsg = { + comm_id: this.commId, + data: msg + }; + + const request = new CommMsgRequest(id, commMsg); + this.session.sendRequest(request); + } +} + +class CommBackendRequest { + kind: 'request' = 'request'; + readonly method: string; + readonly params?: Record; + + private readonly id: string; + + constructor( + private readonly session: KallichoreSession, + private readonly commId: string, + private readonly message: CommRpcMessage, + ) { + this.method = message.method; + this.params = message.params; + + if (!this.message.id) { + throw new Error('Expected `id` field in request'); + } + this.id = this.message.id; + } + + reply(result: any) { + const msg: CommRpcResponse = { + jsonrpc: '2.0', + id: this.id, + method: this.method, + result, + }; + + const commMsg: JupyterCommMsg = { + comm_id: this.commId, + data: msg + }; + + this.session.sendClientMessage(this.commId, this.id, commMsg); + } +} diff --git a/extensions/positron-supervisor/src/RawComm.ts b/extensions/positron-supervisor/src/RawComm.ts new file mode 100644 index 000000000000..9f07015d84af --- /dev/null +++ b/extensions/positron-supervisor/src/RawComm.ts @@ -0,0 +1,44 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2025 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Channel } from './Channel'; + +export type CommBackendMessage = + | { + kind: 'request'; + method: string; + params?: Record; + reply: (result: any) => void; + } + | { + kind: 'notification'; + method: string; + params?: Record; + }; + +export type CommBackendChannel = Channel; + +export interface RawComm { + receiver: CommBackendChannel; + notify: (method: string, params?: Record) => void; + request: (method: string, params?: Record) => Promise; +} + +export interface CommRpcMessage { + jsonrpc: '2.0'; + method: string; + // If present, this indicates a request, otherwise a notification. + // This `id` is otherwise redundant with Jupyter's own `id` field. + id?: string; + params?: Record; + [key: string]: unknown; +} + +export interface CommRpcResponse { + jsonrpc: '2.0'; + result: any; + id: string; + [key: string]: unknown; +} From cb4764a82b89c7432e9e164531ce49f46f683518 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 12 Jun 2025 18:03:43 +0200 Subject: [PATCH 05/48] Clean up client when removed --- extensions/positron-supervisor/src/KallichoreSession.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 84e173f484fa..874faa02829f 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -851,6 +851,8 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { } removeClient(id: string): void { + this._clients.delete(id); + // Ignore this if the session is already exited; an exited session has // no clients if (this._runtimeState === positron.RuntimeState.Exited) { From 21726b82e4e8aa8f191bd357f56e70ee7c7da736 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 13 Jun 2025 08:22:26 +0200 Subject: [PATCH 06/48] Make `RawComm` a disposable --- .../src/KallichoreSession.ts | 45 ++++++++++++------- extensions/positron-supervisor/src/RawComm.ts | 30 +++++++++---- 2 files changed, 52 insertions(+), 23 deletions(-) diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 874faa02829f..35a89419da7f 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -43,7 +43,7 @@ import { DebugRequest } from './jupyter/DebugRequest'; import { JupyterMessageType } from './jupyter/JupyterMessageType.js'; import { Channel } from './Channel'; import { JupyterCommClose } from './jupyter/JupyterCommClose'; -import { CommBackendChannel, CommRpcMessage, CommRpcResponse, RawComm } from './RawComm'; +import { CommBackendMessage, CommRpcMessage, CommRpcResponse, RawComm } from './RawComm'; /** * The reason for a disconnection event. @@ -154,7 +154,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { private readonly _clients: Map = new Map(); /** A map of active comms unmanaged by Positron */ - private readonly _comms: Map = new Map(); + private readonly _comms: Map = new Map(); /** The kernel's log file, if any. */ private _kernelLogFile: string | undefined; @@ -748,9 +748,10 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { ): Promise { const id = `extension-comm-${type}-${this.runtimeMetadata.languageId}-${createUniqueId()}`; - const channel: CommBackendChannel = new Channel(); - this._comms.set(id, channel); - channel.register({ + const comm = new RawCommImpl(id, this); + this._comms.set(id, comm); + + comm.register({ dispose: () => { // If already deleted, it means a `comm_close` from the backend was // received and we don't need to send one. @@ -769,7 +770,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { const commOpen = new CommOpenCommand(msg); await this.sendCommand(commOpen); - return new RawCommImpl(id, this, channel); + return comm; } /** @@ -1941,27 +1942,27 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { // Handle comms that are not managed by Positron first if (msg.header.msg_type === 'comm_close') { const closeMsg = msg.content as JupyterCommClose; - const channel = this._comms.get(closeMsg.comm_id); + const comm = this._comms.get(closeMsg.comm_id); - if (channel) { + if (comm) { // Delete first, this prevents the channel disposable from sending a // `comm_close` back this._comms.delete(closeMsg.comm_id); - channel.dispose(); + comm.dispose(); return; } } if (msg.header.msg_type === 'comm_msg') { const commMsg = msg.content as JupyterCommMsg; - const channel = this._comms.get(commMsg.comm_id); + const comm = this._comms.get(commMsg.comm_id); - if (channel) { + if (comm) { const rpcMsg = commMsg.data as CommRpcMessage; if (rpcMsg.id) { const req = new CommBackendRequest(this, commMsg.comm_id, rpcMsg); - channel.send(req); + comm.receiver.send(req); } else { - channel.send({ kind: 'notification', method: rpcMsg.method, params: rpcMsg.params }); + comm.receiver.send({ kind: 'notification', method: rpcMsg.method, params: rpcMsg.params }); } return; } @@ -2148,11 +2149,13 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { } } -class RawCommImpl { +class RawCommImpl implements vscode.Disposable { + readonly receiver: Channel = new Channel(); + private readonly disposables: vscode.Disposable[] = []; + constructor( private readonly commId: string, private readonly session: KallichoreSession, - public readonly receiver: CommBackendChannel, ) {} notify(method: string, params?: Record) { @@ -2185,6 +2188,18 @@ class RawCommImpl { const request = new CommMsgRequest(id, commMsg); this.session.sendRequest(request); } + + dispose() { + this.receiver.dispose(); + + for (const disposable of this.disposables) { + disposable.dispose(); + } + } + + register(disposable: vscode.Disposable) { + this.disposables.push(disposable); + } } class CommBackendRequest { diff --git a/extensions/positron-supervisor/src/RawComm.ts b/extensions/positron-supervisor/src/RawComm.ts index 9f07015d84af..d410cd605db5 100644 --- a/extensions/positron-supervisor/src/RawComm.ts +++ b/extensions/positron-supervisor/src/RawComm.ts @@ -5,6 +5,28 @@ import { Channel } from './Channel'; +/** + * Raw comm unmanaged by Positron. + * + * This type of comm is not mapped to a Positron client. It lives entirely in + * the extension space and allows private communication between an extension and + * its kernel. + */ +export interface RawComm { + /** Async-iterable for messages sent from backend. */ + receiver: Channel; + + /** Send a notification to the backend comm. */ + notify: (method: string, params?: Record) => void; + + /** Make a request to the backend comm. Resolves when backend responds. */ + request: (method: string, params?: Record) => Promise; + + /** Clear resources and sends `comm_close` to backend comm (unless the channel + * was closed by the backend already). */ + dispose: () => void; +} + export type CommBackendMessage = | { kind: 'request'; @@ -18,14 +40,6 @@ export type CommBackendMessage = params?: Record; }; -export type CommBackendChannel = Channel; - -export interface RawComm { - receiver: CommBackendChannel; - notify: (method: string, params?: Record) => void; - request: (method: string, params?: Record) => Promise; -} - export interface CommRpcMessage { jsonrpc: '2.0'; method: string; From 201d67c156a5ee5210d54beec3dbf9f9600a5bdb Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 13 Jun 2025 08:37:21 +0200 Subject: [PATCH 07/48] Add `reject()` method --- .../src/KallichoreSession.ts | 19 ++++++++++++++++--- extensions/positron-supervisor/src/RawComm.ts | 13 +++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 35a89419da7f..711d2b9eda5e 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -43,7 +43,7 @@ import { DebugRequest } from './jupyter/DebugRequest'; import { JupyterMessageType } from './jupyter/JupyterMessageType.js'; import { Channel } from './Channel'; import { JupyterCommClose } from './jupyter/JupyterCommClose'; -import { CommBackendMessage, CommRpcMessage, CommRpcResponse, RawComm } from './RawComm'; +import { CommBackendMessage, CommRpcError, CommRpcMessage, CommRpcResponse, RawComm } from './RawComm'; /** * The reason for a disconnection event. @@ -2230,12 +2230,25 @@ class CommBackendRequest { method: this.method, result, }; + this.send(msg); + } + + reject(error: Error, code = -32000) { + const msg: CommRpcError = { + jsonrpc: '2.0', + id: this.id, + method: this.method, + message: `${error}`, + code, + }; + this.send(msg); + } + private send(data: Record) { const commMsg: JupyterCommMsg = { comm_id: this.commId, - data: msg + data, }; - this.session.sendClientMessage(this.commId, this.id, commMsg); } } diff --git a/extensions/positron-supervisor/src/RawComm.ts b/extensions/positron-supervisor/src/RawComm.ts index d410cd605db5..cd7594280647 100644 --- a/extensions/positron-supervisor/src/RawComm.ts +++ b/extensions/positron-supervisor/src/RawComm.ts @@ -27,12 +27,17 @@ export interface RawComm { dispose: () => void; } +/** Message from the backend. + * + * If a request, one of the `reply` or `reject` method must be called. + */ export type CommBackendMessage = | { kind: 'request'; method: string; params?: Record; reply: (result: any) => void; + reject: (error: Error) => void; } | { kind: 'notification'; @@ -56,3 +61,11 @@ export interface CommRpcResponse { id: string; [key: string]: unknown; } + +export interface CommRpcError { + jsonrpc: '2.0'; + message: string; + code: number; + id: string; + [key: string]: unknown; +} From e8a41e2385ccee97e2b3d438960de895745d62b4 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 13 Jun 2025 08:54:54 +0200 Subject: [PATCH 08/48] Reorganise files and exports --- .../src/KallichoreSession.ts | 113 +------------- extensions/positron-supervisor/src/RawComm.ts | 142 +++++++++++++----- .../src/positron-supervisor.d.ts | 56 +++++++ 3 files changed, 167 insertions(+), 144 deletions(-) diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 711d2b9eda5e..c1a431b7df7c 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -8,7 +8,7 @@ import * as positron from 'positron'; import * as os from 'os'; import * as path from 'path'; import * as fs from 'fs'; -import { JupyterKernelExtra, JupyterKernelSpec, JupyterLanguageRuntimeSession, JupyterSession } from './positron-supervisor'; +import { CommBackendMessage, JupyterKernelExtra, JupyterKernelSpec, JupyterLanguageRuntimeSession, JupyterSession, RawComm } from './positron-supervisor'; import { ActiveSession, ConnectionInfo, DefaultApi, HttpError, InterruptMode, NewSession, RestartSession, Status, VarAction, VarActionType } from './kcclient/api'; import { JupyterMessage } from './jupyter/JupyterMessage'; import { JupyterRequest } from './jupyter/JupyterRequest'; @@ -43,7 +43,7 @@ import { DebugRequest } from './jupyter/DebugRequest'; import { JupyterMessageType } from './jupyter/JupyterMessageType.js'; import { Channel } from './Channel'; import { JupyterCommClose } from './jupyter/JupyterCommClose'; -import { CommBackendMessage, CommRpcError, CommRpcMessage, CommRpcResponse, RawComm } from './RawComm'; +import { CommBackendRequest, CommRpcMessage, RawCommImpl } from './RawComm'; /** * The reason for a disconnection event. @@ -1958,11 +1958,12 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { if (comm) { const rpcMsg = commMsg.data as CommRpcMessage; + const chan = comm.receiver as Channel; if (rpcMsg.id) { const req = new CommBackendRequest(this, commMsg.comm_id, rpcMsg); - comm.receiver.send(req); + chan.send(req); } else { - comm.receiver.send({ kind: 'notification', method: rpcMsg.method, params: rpcMsg.params }); + chan.send({ kind: 'notification', method: rpcMsg.method, params: rpcMsg.params }); } return; } @@ -2148,107 +2149,3 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { } } } - -class RawCommImpl implements vscode.Disposable { - readonly receiver: Channel = new Channel(); - private readonly disposables: vscode.Disposable[] = []; - - constructor( - private readonly commId: string, - private readonly session: KallichoreSession, - ) {} - - notify(method: string, params?: Record) { - const msg: CommRpcMessage = { - jsonrpc: '2.0', - method, - params, - }; - - // We don't expect a response here, so `id` can be created and forgotten - const id = createUniqueId(); - this.session.sendClientMessage(this.commId, id, msg); - } - - async request(method: string, params?: Record): Promise { - const id = createUniqueId(); - - const msg: CommRpcMessage = { - jsonrpc: '2.0', - id, - method, - params, - }; - - const commMsg: JupyterCommMsg = { - comm_id: this.commId, - data: msg - }; - - const request = new CommMsgRequest(id, commMsg); - this.session.sendRequest(request); - } - - dispose() { - this.receiver.dispose(); - - for (const disposable of this.disposables) { - disposable.dispose(); - } - } - - register(disposable: vscode.Disposable) { - this.disposables.push(disposable); - } -} - -class CommBackendRequest { - kind: 'request' = 'request'; - readonly method: string; - readonly params?: Record; - - private readonly id: string; - - constructor( - private readonly session: KallichoreSession, - private readonly commId: string, - private readonly message: CommRpcMessage, - ) { - this.method = message.method; - this.params = message.params; - - if (!this.message.id) { - throw new Error('Expected `id` field in request'); - } - this.id = this.message.id; - } - - reply(result: any) { - const msg: CommRpcResponse = { - jsonrpc: '2.0', - id: this.id, - method: this.method, - result, - }; - this.send(msg); - } - - reject(error: Error, code = -32000) { - const msg: CommRpcError = { - jsonrpc: '2.0', - id: this.id, - method: this.method, - message: `${error}`, - code, - }; - this.send(msg); - } - - private send(data: Record) { - const commMsg: JupyterCommMsg = { - comm_id: this.commId, - data, - }; - this.session.sendClientMessage(this.commId, this.id, commMsg); - } -} diff --git a/extensions/positron-supervisor/src/RawComm.ts b/extensions/positron-supervisor/src/RawComm.ts index cd7594280647..c18e1a21fb1e 100644 --- a/extensions/positron-supervisor/src/RawComm.ts +++ b/extensions/positron-supervisor/src/RawComm.ts @@ -3,47 +3,117 @@ * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. *--------------------------------------------------------------------------------------------*/ +import * as vscode from 'vscode'; import { Channel } from './Channel'; +import { CommBackendMessage } from './positron-supervisor'; +import { KallichoreSession } from './KallichoreSession'; +import { createUniqueId } from './util'; +import { JupyterCommMsg } from './jupyter/JupyterCommMsg'; +import { CommMsgRequest } from './jupyter/CommMsgRequest'; -/** - * Raw comm unmanaged by Positron. - * - * This type of comm is not mapped to a Positron client. It lives entirely in - * the extension space and allows private communication between an extension and - * its kernel. - */ -export interface RawComm { - /** Async-iterable for messages sent from backend. */ - receiver: Channel; - - /** Send a notification to the backend comm. */ - notify: (method: string, params?: Record) => void; - - /** Make a request to the backend comm. Resolves when backend responds. */ - request: (method: string, params?: Record) => Promise; - - /** Clear resources and sends `comm_close` to backend comm (unless the channel - * was closed by the backend already). */ - dispose: () => void; +export class RawCommImpl implements vscode.Disposable { + readonly receiver: Channel = new Channel(); + private readonly disposables: vscode.Disposable[] = []; + + constructor( + private readonly commId: string, + private readonly session: KallichoreSession, + ) {} + + notify(method: string, params?: Record) { + const msg: CommRpcMessage = { + jsonrpc: '2.0', + method, + params, + }; + + // We don't expect a response here, so `id` can be created and forgotten + const id = createUniqueId(); + this.session.sendClientMessage(this.commId, id, msg); + } + + async request(method: string, params?: Record): Promise { + const id = createUniqueId(); + + const msg: CommRpcMessage = { + jsonrpc: '2.0', + id, + method, + params, + }; + + const commMsg: JupyterCommMsg = { + comm_id: this.commId, + data: msg + }; + + const request = new CommMsgRequest(id, commMsg); + this.session.sendRequest(request); + } + + dispose() { + this.receiver.dispose(); + + for (const disposable of this.disposables) { + disposable.dispose(); + } + } + + register(disposable: vscode.Disposable) { + this.disposables.push(disposable); + } } -/** Message from the backend. - * - * If a request, one of the `reply` or `reject` method must be called. - */ -export type CommBackendMessage = - | { - kind: 'request'; - method: string; - params?: Record; - reply: (result: any) => void; - reject: (error: Error) => void; +export class CommBackendRequest { + kind: 'request' = 'request'; + readonly method: string; + readonly params?: Record; + + private readonly id: string; + + constructor( + private readonly session: KallichoreSession, + private readonly commId: string, + private readonly message: CommRpcMessage, + ) { + this.method = message.method; + this.params = message.params; + + if (!this.message.id) { + throw new Error('Expected `id` field in request'); + } + this.id = this.message.id; + } + + reply(result: any) { + const msg: CommRpcResponse = { + jsonrpc: '2.0', + id: this.id, + method: this.method, + result, + }; + this.send(msg); + } + + reject(error: Error, code = -32000) { + const msg: CommRpcError = { + jsonrpc: '2.0', + id: this.id, + method: this.method, + message: `${error}`, + code, + }; + this.send(msg); } - | { - kind: 'notification'; - method: string; - params?: Record; - }; + + private send(data: Record) { + const commMsg: JupyterCommMsg = { + comm_id: this.commId, + data, + }; + this.session.sendClientMessage(this.commId, this.id, commMsg); + } +} export interface CommRpcMessage { jsonrpc: '2.0'; diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index ee5797600381..26778115c780 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -116,6 +116,17 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS */ createPositronDapClientId(): string; + /** + * Start a raw comm for communication between frontend and backend. + * + * Unlike Positron clients, this kind of comm is private to the calling + * extension and its kernel. + * + * @param debugType Passed as `vscode.DebugConfiguration.type`. + * @param debugName Passed as `vscode.DebugConfiguration.name`. + */ + createComm(type: string, params: Record): Promise; + /** * Method for emitting a message to the language server's Jupyter output * channel. @@ -212,3 +223,48 @@ export interface JupyterKernelExtra { init: (args: Array, delay: number) => void; }; } + +/** + * Raw comm unmanaged by Positron. + * + * This type of comm is not mapped to a Positron client. It lives entirely in + * the extension space and allows private communication between an extension and + * its kernel. + */ +export interface RawComm { + /** Async-iterable for messages sent from backend. */ + receiver: Channel; + + /** Send a notification to the backend comm. */ + notify: (method: string, params?: Record) => void; + + /** Make a request to the backend comm. Resolves when backend responds. */ + request: (method: string, params?: Record) => Promise; + + /** Clear resources and sends `comm_close` to backend comm (unless the channel + * was closed by the backend already). */ + dispose: () => void; +} + +/** + * Communication channel. Dispose to close. + */ +export interface Channel extends AsyncIterable, vscode.Disposable {} + +/** Message from the backend. + * + * If a request, one of the `reply` or `reject` method must be called. + */ +export type CommBackendMessage = + | { + kind: 'request'; + method: string; + params?: Record; + reply: (result: any) => void; + reject: (error: Error) => void; + } + | { + kind: 'notification'; + method: string; + params?: Record; + }; From a0e20886d92b564e77e5de5fc640d30537abe8a0 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 13 Jun 2025 09:11:15 +0200 Subject: [PATCH 09/48] Split channel into tx and rx parts --- extensions/positron-supervisor/src/Channel.ts | 107 ++++++++++++------ .../src/KallichoreSession.ts | 31 +++-- extensions/positron-supervisor/src/RawComm.ts | 4 +- 3 files changed, 91 insertions(+), 51 deletions(-) diff --git a/extensions/positron-supervisor/src/Channel.ts b/extensions/positron-supervisor/src/Channel.ts index eb0072b7a372..96d2bc1e37ec 100644 --- a/extensions/positron-supervisor/src/Channel.ts +++ b/extensions/positron-supervisor/src/Channel.ts @@ -10,66 +10,65 @@ import { delay } from './util'; const YIELD_THRESHOLD = 100; /** - * Multi-consumer multi-producer channel. - * - Used as an async iterator. Dispose to close. - * - All endpoints can close. All are closed at the same time. - * - Sending a value to a closed channel is an error. + * Creates a new channel and returns both sender and receiver. + * Either can be used to close the channel by calling `dispose()`. */ -export class Channel implements AsyncIterable, AsyncIterator, vscode.Disposable { - private closed = false; - private queue: T[] = []; - private pending_consumers: ((value: IteratorResult) => void)[] = []; - private i = 0; +export function channel(): [Sender, Receiver] { + const state = new ChannelState(); + const sender = new Sender(state); + const receiver = new Receiver(state); + return [sender, receiver]; +} + +/** + * Channel sender (tx). synchronously sends values to the channel. + */ +export class Sender implements vscode.Disposable { private disposables: vscode.Disposable[] = []; - private isDisposed = false; + + constructor(private state: ChannelState) {} send(value: T) { - if (this.closed) { + if (this.state.closed) { throw new Error('Can\'t send values after channel is closed'); } - if (this.pending_consumers.length > 0) { + if (this.state.pending_consumers.length > 0) { // There is a consumer waiting, resolve it immediately - this.pending_consumers.shift()!({ value, done: false }); + this.state.pending_consumers.shift()!({ value, done: false }); } else { // No consumer waiting, queue up the value - this.queue.push(value); + this.state.queue.push(value); } } - private close() { - this.closed = true; - - // Resolve all pending consumers as done - while (this.pending_consumers.length > 0) { - this.pending_consumers.shift()!({ value: undefined, done: true }); - } - } - - dispose() { - // Since channel is owned by multiple endpoints we need to be careful about - // `dispose()` being idempotent - if (this.isDisposed) { - return; - } - this.isDisposed = true; - - this.close(); + dispose() { + this.state.dispose(); for (const disposable of this.disposables) { disposable.dispose(); } - } + } register(disposable: vscode.Disposable) { this.disposables.push(disposable); } +} + +/** + * Channel receiver (rx). Async-iterable to receive values from the channel. + */ +export class Receiver implements AsyncIterable, AsyncIterator, vscode.Disposable { + private i = 0; + private disposables: vscode.Disposable[] = []; + + constructor(private state: ChannelState) {} [Symbol.asyncIterator]() { return this; } async next(): Promise> { - if (this.queue.length > 0) { + if (this.state.queue.length > 0) { ++this.i; // Yield regularly to event loop to avoid starvation. Sends are @@ -79,17 +78,51 @@ export class Channel implements AsyncIterable, AsyncIterator, vscode.Di await delay(0); } - return { value: this.queue.shift()!, done: false }; + return { value: this.state.queue.shift()!, done: false }; } // If nothing in the queue and the channel is closed, we're done - if (this.closed) { + if (this.state.closed) { return { value: undefined, done: true }; } // Nothing in the queue, wait for a value to be sent return new Promise>((resolve) => { - this.pending_consumers.push(resolve); + this.state.pending_consumers.push(resolve); }); } + + dispose() { + this.state.dispose(); + for (const disposable of this.disposables) { + disposable.dispose(); + } + } + + register(disposable: vscode.Disposable) { + this.disposables.push(disposable); + } +} + +/** + * Shared state between sender and receiver + */ +class ChannelState { + closed = false; + queue: T[] = []; + pending_consumers: ((value: IteratorResult) => void)[] = []; + + dispose() { + // Since channel is owned by multiple endpoints we need to be careful about + // `dispose()` being idempotent + if (this.closed) { + return; + } + this.closed = true; + + // Resolve all pending consumers as done + while (this.pending_consumers.length > 0) { + this.pending_consumers.shift()!({ value: undefined, done: true }); + } + } } diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index c1a431b7df7c..d3ab847e531b 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -41,7 +41,7 @@ import { createUniqueId, summarizeError, summarizeHttpError } from './util'; import { AdoptedSession } from './AdoptedSession'; import { DebugRequest } from './jupyter/DebugRequest'; import { JupyterMessageType } from './jupyter/JupyterMessageType.js'; -import { Channel } from './Channel'; +import { channel, Sender } from './Channel'; import { JupyterCommClose } from './jupyter/JupyterCommClose'; import { CommBackendRequest, CommRpcMessage, RawCommImpl } from './RawComm'; @@ -154,7 +154,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { private readonly _clients: Map = new Map(); /** A map of active comms unmanaged by Positron */ - private readonly _comms: Map = new Map(); + private readonly _comms: Map]> = new Map(); /** The kernel's log file, if any. */ private _kernelLogFile: string | undefined; @@ -748,9 +748,11 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { ): Promise { const id = `extension-comm-${type}-${this.runtimeMetadata.languageId}-${createUniqueId()}`; - const comm = new RawCommImpl(id, this); - this._comms.set(id, comm); + const [tx, rx] = channel(); + const comm = new RawCommImpl(id, this, rx); + this._comms.set(id, [comm, tx]); + // Disposal handler that allows extension to initiate close comm comm.register({ dispose: () => { // If already deleted, it means a `comm_close` from the backend was @@ -1942,29 +1944,34 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { // Handle comms that are not managed by Positron first if (msg.header.msg_type === 'comm_close') { const closeMsg = msg.content as JupyterCommClose; - const comm = this._comms.get(closeMsg.comm_id); + const commHandle = this._comms.get(closeMsg.comm_id); - if (comm) { + if (commHandle) { // Delete first, this prevents the channel disposable from sending a // `comm_close` back this._comms.delete(closeMsg.comm_id); + + const [comm, _] = commHandle; comm.dispose(); + return; } } + if (msg.header.msg_type === 'comm_msg') { const commMsg = msg.content as JupyterCommMsg; - const comm = this._comms.get(commMsg.comm_id); + const commHandle = this._comms.get(commMsg.comm_id); - if (comm) { + if (commHandle) { + const [_, tx] = commHandle; const rpcMsg = commMsg.data as CommRpcMessage; - const chan = comm.receiver as Channel; + if (rpcMsg.id) { - const req = new CommBackendRequest(this, commMsg.comm_id, rpcMsg); - chan.send(req); + tx.send(new CommBackendRequest(this, commMsg.comm_id, rpcMsg)); } else { - chan.send({ kind: 'notification', method: rpcMsg.method, params: rpcMsg.params }); + tx.send({ kind: 'notification', method: rpcMsg.method, params: rpcMsg.params }); } + return; } } diff --git a/extensions/positron-supervisor/src/RawComm.ts b/extensions/positron-supervisor/src/RawComm.ts index c18e1a21fb1e..56ead1de059f 100644 --- a/extensions/positron-supervisor/src/RawComm.ts +++ b/extensions/positron-supervisor/src/RawComm.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as vscode from 'vscode'; -import { Channel } from './Channel'; +import { Receiver } from './Channel'; import { CommBackendMessage } from './positron-supervisor'; import { KallichoreSession } from './KallichoreSession'; import { createUniqueId } from './util'; @@ -12,12 +12,12 @@ import { JupyterCommMsg } from './jupyter/JupyterCommMsg'; import { CommMsgRequest } from './jupyter/CommMsgRequest'; export class RawCommImpl implements vscode.Disposable { - readonly receiver: Channel = new Channel(); private readonly disposables: vscode.Disposable[] = []; constructor( private readonly commId: string, private readonly session: KallichoreSession, + public readonly receiver: Receiver, ) {} notify(method: string, params?: Record) { From 8b89219a24d1641a94d75e2808859dda31d00da3 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 13 Jun 2025 09:48:15 +0200 Subject: [PATCH 10/48] Return boolean from `send()` to indicate whether channel was still open --- extensions/positron-supervisor/src/Channel.ts | 11 +++++++++-- .../positron-supervisor/src/KallichoreSession.ts | 1 + 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/extensions/positron-supervisor/src/Channel.ts b/extensions/positron-supervisor/src/Channel.ts index 96d2bc1e37ec..b92b56268da6 100644 --- a/extensions/positron-supervisor/src/Channel.ts +++ b/extensions/positron-supervisor/src/Channel.ts @@ -28,9 +28,14 @@ export class Sender implements vscode.Disposable { constructor(private state: ChannelState) {} - send(value: T) { + /** + * Sends a value to the channel. + * @param value A value to send + * @returns `true` if the value was sent successfully, `false` if the channel is closed. + */ + send(value: T): boolean { if (this.state.closed) { - throw new Error('Can\'t send values after channel is closed'); + return false; } if (this.state.pending_consumers.length > 0) { @@ -40,6 +45,8 @@ export class Sender implements vscode.Disposable { // No consumer waiting, queue up the value this.state.queue.push(value); } + + return true; } dispose() { diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index d3ab847e531b..c6aafd811bd4 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -1963,6 +1963,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { const commHandle = this._comms.get(commMsg.comm_id); if (commHandle) { + // Channel is still opened, go ahead and forward message const [_, tx] = commHandle; const rpcMsg = commMsg.data as CommRpcMessage; From cd14914155a459abeb8820ccfb81afc83cf5e5f8 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 13 Jun 2025 10:53:54 +0200 Subject: [PATCH 11/48] Take handlers on comm creation instead of returning channel --- .../src/KallichoreSession.ts | 44 +++------ extensions/positron-supervisor/src/RawComm.ts | 94 +++++++++---------- .../src/positron-supervisor.d.ts | 39 ++------ 3 files changed, 72 insertions(+), 105 deletions(-) diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index c6aafd811bd4..07d6f0f9c15c 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -8,7 +8,7 @@ import * as positron from 'positron'; import * as os from 'os'; import * as path from 'path'; import * as fs from 'fs'; -import { CommBackendMessage, JupyterKernelExtra, JupyterKernelSpec, JupyterLanguageRuntimeSession, JupyterSession, RawComm } from './positron-supervisor'; +import { JupyterKernelExtra, JupyterKernelSpec, JupyterLanguageRuntimeSession, JupyterSession, RawComm } from './positron-supervisor'; import { ActiveSession, ConnectionInfo, DefaultApi, HttpError, InterruptMode, NewSession, RestartSession, Status, VarAction, VarActionType } from './kcclient/api'; import { JupyterMessage } from './jupyter/JupyterMessage'; import { JupyterRequest } from './jupyter/JupyterRequest'; @@ -41,9 +41,8 @@ import { createUniqueId, summarizeError, summarizeHttpError } from './util'; import { AdoptedSession } from './AdoptedSession'; import { DebugRequest } from './jupyter/DebugRequest'; import { JupyterMessageType } from './jupyter/JupyterMessageType.js'; -import { channel, Sender } from './Channel'; import { JupyterCommClose } from './jupyter/JupyterCommClose'; -import { CommBackendRequest, CommRpcMessage, RawCommImpl } from './RawComm'; +import { RawCommImpl } from './RawComm'; /** * The reason for a disconnection event. @@ -154,7 +153,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { private readonly _clients: Map = new Map(); /** A map of active comms unmanaged by Positron */ - private readonly _comms: Map]> = new Map(); + private readonly _comms: Map = new Map(); /** The kernel's log file, if any. */ private _kernelLogFile: string | undefined; @@ -744,13 +743,14 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { async createComm( type: string, - params: Record, - ): Promise { + handleNotification: (method: string, params?: Record) => void, + handleRequest: (method: string, params?: Record) => any, + params: Record = {}, + ): Promise { const id = `extension-comm-${type}-${this.runtimeMetadata.languageId}-${createUniqueId()}`; - const [tx, rx] = channel(); - const comm = new RawCommImpl(id, this, rx); - this._comms.set(id, [comm, tx]); + const comm = new RawCommImpl(id, this, handleNotification, handleRequest); + this._comms.set(id, comm); // Disposal handler that allows extension to initiate close comm comm.register({ @@ -767,7 +767,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { const msg: JupyterCommOpen = { target_name: type, comm_id: id, - data: params + data: params, }; const commOpen = new CommOpenCommand(msg); await this.sendCommand(commOpen); @@ -1944,35 +1944,21 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { // Handle comms that are not managed by Positron first if (msg.header.msg_type === 'comm_close') { const closeMsg = msg.content as JupyterCommClose; - const commHandle = this._comms.get(closeMsg.comm_id); + const comm = this._comms.get(closeMsg.comm_id); - if (commHandle) { - // Delete first, this prevents the channel disposable from sending a - // `comm_close` back + if (comm) { this._comms.delete(closeMsg.comm_id); - - const [comm, _] = commHandle; comm.dispose(); - return; } } if (msg.header.msg_type === 'comm_msg') { const commMsg = msg.content as JupyterCommMsg; - const commHandle = this._comms.get(commMsg.comm_id); - - if (commHandle) { - // Channel is still opened, go ahead and forward message - const [_, tx] = commHandle; - const rpcMsg = commMsg.data as CommRpcMessage; - - if (rpcMsg.id) { - tx.send(new CommBackendRequest(this, commMsg.comm_id, rpcMsg)); - } else { - tx.send({ kind: 'notification', method: rpcMsg.method, params: rpcMsg.params }); - } + const comm = this._comms.get(commMsg.comm_id); + if (comm) { + comm.handleMessage(commMsg); return; } } diff --git a/extensions/positron-supervisor/src/RawComm.ts b/extensions/positron-supervisor/src/RawComm.ts index 56ead1de059f..abb3ec08b9a5 100644 --- a/extensions/positron-supervisor/src/RawComm.ts +++ b/extensions/positron-supervisor/src/RawComm.ts @@ -4,8 +4,6 @@ *--------------------------------------------------------------------------------------------*/ import * as vscode from 'vscode'; -import { Receiver } from './Channel'; -import { CommBackendMessage } from './positron-supervisor'; import { KallichoreSession } from './KallichoreSession'; import { createUniqueId } from './util'; import { JupyterCommMsg } from './jupyter/JupyterCommMsg'; @@ -17,7 +15,8 @@ export class RawCommImpl implements vscode.Disposable { constructor( private readonly commId: string, private readonly session: KallichoreSession, - public readonly receiver: Receiver, + private readonly onNotification: (method: string, params?: Record) => void, + private readonly onRequest: (method: string, params?: Record) => any, ) {} notify(method: string, params?: Record) { @@ -51,71 +50,72 @@ export class RawCommImpl implements vscode.Disposable { this.session.sendRequest(request); } - dispose() { - this.receiver.dispose(); - - for (const disposable of this.disposables) { - disposable.dispose(); - } - } - - register(disposable: vscode.Disposable) { - this.disposables.push(disposable); - } -} - -export class CommBackendRequest { - kind: 'request' = 'request'; - readonly method: string; - readonly params?: Record; - - private readonly id: string; - - constructor( - private readonly session: KallichoreSession, - private readonly commId: string, - private readonly message: CommRpcMessage, - ) { - this.method = message.method; - this.params = message.params; - - if (!this.message.id) { - throw new Error('Expected `id` field in request'); + // Relay message from backend to extension + handleMessage(message: JupyterCommMsg) { + const data = message.data; + const rpc = data as CommRpcMessage; + const isNotification = rpc.id === undefined; + + try { + if (isNotification) { + this.onNotification(rpc.method, rpc.params); + } else { + const result = this.onRequest(rpc.method, rpc.params); + this.reply(rpc.id!, rpc.method, result); + } + } catch (err) { + if (isNotification) { + this.session.log( + `Notification handler for ${message.comm_id} failed: ${err}`, + vscode.LogLevel.Warning + ); + } else { + this.reject(rpc.id!, rpc.method, err); + } } - this.id = this.message.id; } - reply(result: any) { + private reply(id: string, method: string, result: any) { const msg: CommRpcResponse = { jsonrpc: '2.0', - id: this.id, - method: this.method, + id, + method, result, }; - this.send(msg); + this.send(id, msg); } - reject(error: Error, code = -32000) { + private reject(id: string, method: string, error: Error, code = -32000) { const msg: CommRpcError = { jsonrpc: '2.0', - id: this.id, - method: this.method, + id, + method, message: `${error}`, code, }; - this.send(msg); + this.send(id, msg); } - private send(data: Record) { + private send(id: string, data: Record) { const commMsg: JupyterCommMsg = { comm_id: this.commId, data, }; - this.session.sendClientMessage(this.commId, this.id, commMsg); + this.session.sendClientMessage(this.commId, id, commMsg); + } + + dispose() { + for (const disposable of this.disposables) { + disposable.dispose(); + } + } + + register(disposable: vscode.Disposable) { + this.disposables.push(disposable); } } -export interface CommRpcMessage { +interface CommRpcMessage { jsonrpc: '2.0'; method: string; // If present, this indicates a request, otherwise a notification. @@ -125,14 +125,14 @@ export interface CommRpcMessage { [key: string]: unknown; } -export interface CommRpcResponse { +interface CommRpcResponse { jsonrpc: '2.0'; result: any; id: string; [key: string]: unknown; } -export interface CommRpcError { +interface CommRpcError { jsonrpc: '2.0'; message: string; code: number; diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index 26778115c780..a9dfed98c895 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -122,10 +122,17 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * Unlike Positron clients, this kind of comm is private to the calling * extension and its kernel. * - * @param debugType Passed as `vscode.DebugConfiguration.type`. - * @param debugName Passed as `vscode.DebugConfiguration.name`. + * @param type Comm type, used to generate comm identifier. + * @param handleNotification Handle notification from backend. + * @param handleRequest Handle request from backend. Throw error to reject. + * @param params Optionally, additional parameters included in `comm_open`. */ - createComm(type: string, params: Record): Promise; + createComm( + type: string, + handleNotification: (method: string, params?: Record) => void, + handleRequest: (method: string, params?: Record) => any, + params?: Record, + ): Promise; /** * Method for emitting a message to the language server's Jupyter output @@ -232,9 +239,6 @@ export interface JupyterKernelExtra { * its kernel. */ export interface RawComm { - /** Async-iterable for messages sent from backend. */ - receiver: Channel; - /** Send a notification to the backend comm. */ notify: (method: string, params?: Record) => void; @@ -245,26 +249,3 @@ export interface RawComm { * was closed by the backend already). */ dispose: () => void; } - -/** - * Communication channel. Dispose to close. - */ -export interface Channel extends AsyncIterable, vscode.Disposable {} - -/** Message from the backend. - * - * If a request, one of the `reply` or `reject` method must be called. - */ -export type CommBackendMessage = - | { - kind: 'request'; - method: string; - params?: Record; - reply: (result: any) => void; - reject: (error: Error) => void; - } - | { - kind: 'notification'; - method: string; - params?: Record; - }; From 73036e0e4e8fec9748fd6fc0cfdd9d676663dc81 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 13 Jun 2025 11:39:53 +0200 Subject: [PATCH 12/48] Streamline comm closing --- .../src/KallichoreSession.ts | 10 +++++++-- extensions/positron-supervisor/src/RawComm.ts | 22 ++++++++++++++++--- .../src/positron-supervisor.d.ts | 14 ++++++++---- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 07d6f0f9c15c..778e0047b5d2 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -1792,9 +1792,15 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { this._connected = new Barrier(); } - // All comms are now closed + // All clients are now closed this._clients.clear(); + // Close all raw comms + for (const comm of this._comms.values()) { + comm.close(); + } + this._comms.clear(); + // Clear any starting comms this._startingComms.forEach((promise) => { promise.reject(new Error('Kernel exited')); @@ -1948,7 +1954,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { if (comm) { this._comms.delete(closeMsg.comm_id); - comm.dispose(); + comm.close(); return; } } diff --git a/extensions/positron-supervisor/src/RawComm.ts b/extensions/positron-supervisor/src/RawComm.ts index abb3ec08b9a5..08218bc9e91a 100644 --- a/extensions/positron-supervisor/src/RawComm.ts +++ b/extensions/positron-supervisor/src/RawComm.ts @@ -11,6 +11,7 @@ import { CommMsgRequest } from './jupyter/CommMsgRequest'; export class RawCommImpl implements vscode.Disposable { private readonly disposables: vscode.Disposable[] = []; + private closed = false; constructor( private readonly commId: string, @@ -19,7 +20,11 @@ export class RawCommImpl implements vscode.Disposable { private readonly onRequest: (method: string, params?: Record) => any, ) {} - notify(method: string, params?: Record) { + notify(method: string, params?: Record): boolean { + if (this.closed) { + return false; + } + const msg: CommRpcMessage = { jsonrpc: '2.0', method, @@ -29,9 +34,15 @@ export class RawCommImpl implements vscode.Disposable { // We don't expect a response here, so `id` can be created and forgotten const id = createUniqueId(); this.session.sendClientMessage(this.commId, id, msg); + + return true; } - async request(method: string, params?: Record): Promise { + async request(method: string, params?: Record): Promise<[boolean, any]> { + if (this.closed) { + return [false, undefined]; + } + const id = createUniqueId(); const msg: CommRpcMessage = { @@ -47,7 +58,7 @@ export class RawCommImpl implements vscode.Disposable { }; const request = new CommMsgRequest(id, commMsg); - this.session.sendRequest(request); + return [true, this.session.sendRequest(request)]; } // Relay message from backend to extension @@ -104,7 +115,12 @@ export class RawCommImpl implements vscode.Disposable { this.session.sendClientMessage(this.commId, id, commMsg); } + close() { + this.closed = true; + } + dispose() { + this.close(); for (const disposable of this.disposables) { disposable.dispose(); } diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index a9dfed98c895..1dfd83cd4334 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -237,13 +237,19 @@ export interface JupyterKernelExtra { * This type of comm is not mapped to a Positron client. It lives entirely in * the extension space and allows private communication between an extension and * its kernel. + * + * It's a disposable. Dispose of it once it's closed or you're no longer using + * it. If the comm has not already been closed by the kernel, a client-initiated + * `comm_close` message is emitted. */ export interface RawComm { - /** Send a notification to the backend comm. */ - notify: (method: string, params?: Record) => void; + /** Send a notification to the backend comm. Returns `false` if comm was closed. */ + notify: (method: string, params?: Record) => boolean; - /** Make a request to the backend comm. Resolves when backend responds. */ - request: (method: string, params?: Record) => Promise; + /** Make a request to the backend comm. Resolves when backend responds. The tuple's + * first value indicates whether the comm was closed (and no request was emitted). + * The second value is the result if the request was made. */ + request: (method: string, params?: Record) => Promise<[boolean, any]>; /** Clear resources and sends `comm_close` to backend comm (unless the channel * was closed by the backend already). */ From dcefb60cea23762708aa55372b2b248f5e0dac89 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 13 Jun 2025 12:57:42 +0200 Subject: [PATCH 13/48] Use `switch` --- .../src/KallichoreSession.ts | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 778e0047b5d2..b35666199218 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -1948,24 +1948,28 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { } // Handle comms that are not managed by Positron first - if (msg.header.msg_type === 'comm_close') { - const closeMsg = msg.content as JupyterCommClose; - const comm = this._comms.get(closeMsg.comm_id); - - if (comm) { - this._comms.delete(closeMsg.comm_id); - comm.close(); - return; + switch (msg.header.msg_type) { + case 'comm_close': { + const closeMsg = msg.content as JupyterCommClose; + const comm = this._comms.get(closeMsg.comm_id); + + if (comm) { + this._comms.delete(closeMsg.comm_id); + comm.close(); + return; + } + break; } - } - if (msg.header.msg_type === 'comm_msg') { - const commMsg = msg.content as JupyterCommMsg; - const comm = this._comms.get(commMsg.comm_id); + case 'comm_msg': { + const commMsg = msg.content as JupyterCommMsg; + const comm = this._comms.get(commMsg.comm_id); - if (comm) { - comm.handleMessage(commMsg); - return; + if (comm) { + comm.handleMessage(commMsg); + return; + } + break; } } From 67bc9e479f081292008976ae1daec6bf0d5268a0 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 13 Jun 2025 13:46:42 +0200 Subject: [PATCH 14/48] Use standard `target_name` parameter name --- extensions/positron-supervisor/src/KallichoreSession.ts | 6 +++--- extensions/positron-supervisor/src/positron-supervisor.d.ts | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index b35666199218..66d5c4d9fb18 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -742,12 +742,12 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { } async createComm( - type: string, + target_name: string, handleNotification: (method: string, params?: Record) => void, handleRequest: (method: string, params?: Record) => any, params: Record = {}, ): Promise { - const id = `extension-comm-${type}-${this.runtimeMetadata.languageId}-${createUniqueId()}`; + const id = `extension-comm-${target_name}-${this.runtimeMetadata.languageId}-${createUniqueId()}`; const comm = new RawCommImpl(id, this, handleNotification, handleRequest); this._comms.set(id, comm); @@ -765,7 +765,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { }); const msg: JupyterCommOpen = { - target_name: type, + target_name, comm_id: id, data: params, }; diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index 1dfd83cd4334..c830d9d4e7c2 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -122,13 +122,13 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * Unlike Positron clients, this kind of comm is private to the calling * extension and its kernel. * - * @param type Comm type, used to generate comm identifier. + * @param target_name Comm type, also used to generate comm identifier. * @param handleNotification Handle notification from backend. * @param handleRequest Handle request from backend. Throw error to reject. * @param params Optionally, additional parameters included in `comm_open`. */ createComm( - type: string, + target_name: string, handleNotification: (method: string, params?: Record) => void, handleRequest: (method: string, params?: Record) => any, params?: Record, From 698028ec678fc24ca417c8d2b16a8536a596581a Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 16 Jun 2025 10:37:37 +0200 Subject: [PATCH 15/48] Revert to async-iterable approach after all --- .../src/KallichoreSession.ts | 46 +++++--- extensions/positron-supervisor/src/RawComm.ts | 109 ++++++++++-------- .../src/positron-supervisor.d.ts | 29 ++++- 3 files changed, 116 insertions(+), 68 deletions(-) diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 66d5c4d9fb18..45cb06e25689 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -8,7 +8,7 @@ import * as positron from 'positron'; import * as os from 'os'; import * as path from 'path'; import * as fs from 'fs'; -import { JupyterKernelExtra, JupyterKernelSpec, JupyterLanguageRuntimeSession, JupyterSession, RawComm } from './positron-supervisor'; +import { CommBackendMessage, JupyterKernelExtra, JupyterKernelSpec, JupyterLanguageRuntimeSession, JupyterSession, RawComm } from './positron-supervisor'; import { ActiveSession, ConnectionInfo, DefaultApi, HttpError, InterruptMode, NewSession, RestartSession, Status, VarAction, VarActionType } from './kcclient/api'; import { JupyterMessage } from './jupyter/JupyterMessage'; import { JupyterRequest } from './jupyter/JupyterRequest'; @@ -42,7 +42,8 @@ import { AdoptedSession } from './AdoptedSession'; import { DebugRequest } from './jupyter/DebugRequest'; import { JupyterMessageType } from './jupyter/JupyterMessageType.js'; import { JupyterCommClose } from './jupyter/JupyterCommClose'; -import { RawCommImpl } from './RawComm'; +import { CommBackendRequest, CommRpcMessage, RawCommImpl } from './RawComm'; +import { channel, Sender } from './Channel'; /** * The reason for a disconnection event. @@ -153,7 +154,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { private readonly _clients: Map = new Map(); /** A map of active comms unmanaged by Positron */ - private readonly _comms: Map = new Map(); + private readonly _comms: Map]> = new Map(); /** The kernel's log file, if any. */ private _kernelLogFile: string | undefined; @@ -743,14 +744,13 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { async createComm( target_name: string, - handleNotification: (method: string, params?: Record) => void, - handleRequest: (method: string, params?: Record) => any, params: Record = {}, ): Promise { const id = `extension-comm-${target_name}-${this.runtimeMetadata.languageId}-${createUniqueId()}`; - const comm = new RawCommImpl(id, this, handleNotification, handleRequest); - this._comms.set(id, comm); + const [tx, rx] = channel(); + const comm = new RawCommImpl(id, this, rx); + this._comms.set(id, [comm, tx]); // Disposal handler that allows extension to initiate close comm comm.register({ @@ -758,8 +758,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { // If already deleted, it means a `comm_close` from the backend was // received and we don't need to send one. if (this._comms.delete(id)) { - const commClose = new CommCloseCommand(id); - this.sendCommand(commClose); + comm.close_and_notify(); } } }); @@ -1796,8 +1795,10 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { this._clients.clear(); // Close all raw comms - for (const comm of this._comms.values()) { - comm.close(); + for (const [comm, tx] of this._comms.values()) { + // Don't dispose of comm, this resource is owned by caller of `createComm()`. + (comm as RawCommImpl).close(); + tx.dispose(); } this._comms.clear(); @@ -1951,11 +1952,15 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { switch (msg.header.msg_type) { case 'comm_close': { const closeMsg = msg.content as JupyterCommClose; - const comm = this._comms.get(closeMsg.comm_id); + const commHandle = this._comms.get(closeMsg.comm_id); - if (comm) { + if (commHandle) { + // Delete first, this prevents the channel disposable from sending a + // `comm_close` back this._comms.delete(closeMsg.comm_id); - comm.close(); + + const [comm, _] = commHandle; + comm.dispose(); return; } break; @@ -1963,10 +1968,17 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { case 'comm_msg': { const commMsg = msg.content as JupyterCommMsg; - const comm = this._comms.get(commMsg.comm_id); + const commHandle = this._comms.get(commMsg.comm_id); + + if (commHandle) { + const [_, tx] = commHandle; + const rpcMsg = commMsg.data as CommRpcMessage; - if (comm) { - comm.handleMessage(commMsg); + if (rpcMsg.id) { + tx.send(new CommBackendRequest(this, commMsg.comm_id, rpcMsg)); + } else { + tx.send({ kind: 'notification', method: rpcMsg.method, params: rpcMsg.params }); + } return; } break; diff --git a/extensions/positron-supervisor/src/RawComm.ts b/extensions/positron-supervisor/src/RawComm.ts index 08218bc9e91a..affaaa21e511 100644 --- a/extensions/positron-supervisor/src/RawComm.ts +++ b/extensions/positron-supervisor/src/RawComm.ts @@ -8,6 +8,9 @@ import { KallichoreSession } from './KallichoreSession'; import { createUniqueId } from './util'; import { JupyterCommMsg } from './jupyter/JupyterCommMsg'; import { CommMsgRequest } from './jupyter/CommMsgRequest'; +import { Receiver } from './Channel'; +import { CommBackendMessage } from './positron-supervisor'; +import { CommCloseCommand } from './jupyter/CommCloseCommand'; export class RawCommImpl implements vscode.Disposable { private readonly disposables: vscode.Disposable[] = []; @@ -16,8 +19,7 @@ export class RawCommImpl implements vscode.Disposable { constructor( private readonly commId: string, private readonly session: KallichoreSession, - private readonly onNotification: (method: string, params?: Record) => void, - private readonly onRequest: (method: string, params?: Record) => any, + public readonly receiver: Receiver, ) {} notify(method: string, params?: Record): boolean { @@ -61,77 +63,90 @@ export class RawCommImpl implements vscode.Disposable { return [true, this.session.sendRequest(request)]; } - // Relay message from backend to extension - handleMessage(message: JupyterCommMsg) { - const data = message.data; - const rpc = data as CommRpcMessage; - const isNotification = rpc.id === undefined; + close() { + this.closed = true; + } + + close_and_notify() { + this.close(); + const commClose = new CommCloseCommand(this.commId); + this.session.sendCommand(commClose); + } + + dispose() { + this.close(); + for (const disposable of this.disposables) { + disposable.dispose(); + } + } + + register(disposable: vscode.Disposable) { + this.disposables.push(disposable); + } +} + +export class CommBackendRequest { + kind: 'request' = 'request'; + readonly method: string; + readonly params?: Record; + + private readonly id: string; + + constructor( + private readonly session: KallichoreSession, + private readonly commId: string, + private readonly message: CommRpcMessage, + ) { + this.method = message.method; + this.params = message.params; + + if (!this.message.id) { + throw new Error('Expected `id` field in request'); + } + this.id = this.message.id; + } + // Handle request. Takes a callback and responds with return value or rejects + // with error if one is thrown. + handle(handler: () => any) { try { - if (isNotification) { - this.onNotification(rpc.method, rpc.params); - } else { - const result = this.onRequest(rpc.method, rpc.params); - this.reply(rpc.id!, rpc.method, result); - } + this.reply(handler()); } catch (err) { - if (isNotification) { - this.session.log( - `Notification handler for ${message.comm_id} failed: ${err}`, - vscode.LogLevel.Warning - ); - } else { - this.reject(rpc.id!, rpc.method, err); - } + this.reject(err); } } - private reply(id: string, method: string, result: any) { + reply(result: any) { const msg: CommRpcResponse = { jsonrpc: '2.0', - id, - method, + id: this.id, + method: this.method, result, }; - this.send(id, msg); + this.send(msg); } - private reject(id: string, method: string, error: Error, code = -32000) { + reject(error: Error, code = -32000) { const msg: CommRpcError = { jsonrpc: '2.0', - id, - method, + id: this.id, + method: this.method, message: `${error}`, code, }; - this.send(id, msg); + this.send(msg); } - private send(id: string, data: Record) { + private send(data: Record) { const commMsg: JupyterCommMsg = { comm_id: this.commId, data, }; - this.session.sendClientMessage(this.commId, id, commMsg); - } - - close() { - this.closed = true; - } - - dispose() { - this.close(); - for (const disposable of this.disposables) { - disposable.dispose(); - } - } - - register(disposable: vscode.Disposable) { - this.disposables.push(disposable); + this.session.sendClientMessage(this.commId, this.id, commMsg); } } -interface CommRpcMessage { +export interface CommRpcMessage { jsonrpc: '2.0'; method: string; // If present, this indicates a request, otherwise a notification. diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index c830d9d4e7c2..3604309ae70c 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -123,14 +123,10 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * extension and its kernel. * * @param target_name Comm type, also used to generate comm identifier. - * @param handleNotification Handle notification from backend. - * @param handleRequest Handle request from backend. Throw error to reject. * @param params Optionally, additional parameters included in `comm_open`. */ createComm( target_name: string, - handleNotification: (method: string, params?: Record) => void, - handleRequest: (method: string, params?: Record) => any, params?: Record, ): Promise; @@ -243,6 +239,9 @@ export interface JupyterKernelExtra { * `comm_close` message is emitted. */ export interface RawComm { + /** Async-iterable for messages sent from backend. */ + receiver: Channel; + /** Send a notification to the backend comm. Returns `false` if comm was closed. */ notify: (method: string, params?: Record) => boolean; @@ -255,3 +254,25 @@ export interface RawComm { * was closed by the backend already). */ dispose: () => void; } + +/** + * Communication channel. Dispose to close. + */ +export interface Channel extends AsyncIterable, vscode.Disposable {} + +/** Message from the backend. + * + * If a request, the `handle` method _must_ be called. + */ +export type CommBackendMessage = + | { + kind: 'request'; + method: string; + params?: Record; + handle: (handler: () => any) => void; + } + | { + kind: 'notification'; + method: string; + params?: Record; + }; From bf7f35912ed2ddfccb8bd116dbbaf20ee63052f7 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Sun, 10 Aug 2025 05:03:22 -0400 Subject: [PATCH 16/48] Make comm ID public --- extensions/positron-supervisor/src/RawComm.ts | 10 +++++----- .../positron-supervisor/src/positron-supervisor.d.ts | 9 ++++++--- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/extensions/positron-supervisor/src/RawComm.ts b/extensions/positron-supervisor/src/RawComm.ts index affaaa21e511..2d67b788f50c 100644 --- a/extensions/positron-supervisor/src/RawComm.ts +++ b/extensions/positron-supervisor/src/RawComm.ts @@ -17,10 +17,10 @@ export class RawCommImpl implements vscode.Disposable { private closed = false; constructor( - private readonly commId: string, + public readonly id: string, private readonly session: KallichoreSession, public readonly receiver: Receiver, - ) {} + ) { } notify(method: string, params?: Record): boolean { if (this.closed) { @@ -35,7 +35,7 @@ export class RawCommImpl implements vscode.Disposable { // We don't expect a response here, so `id` can be created and forgotten const id = createUniqueId(); - this.session.sendClientMessage(this.commId, id, msg); + this.session.sendClientMessage(this.id, id, msg); return true; } @@ -55,7 +55,7 @@ export class RawCommImpl implements vscode.Disposable { }; const commMsg: JupyterCommMsg = { - comm_id: this.commId, + comm_id: this.id, data: msg }; @@ -69,7 +69,7 @@ export class RawCommImpl implements vscode.Disposable { close_and_notify() { this.close(); - const commClose = new CommCloseCommand(this.commId); + const commClose = new CommCloseCommand(this.id); this.session.sendCommand(commClose); } diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index 3604309ae70c..e839a5906aa2 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -239,6 +239,9 @@ export interface JupyterKernelExtra { * `comm_close` message is emitted. */ export interface RawComm { + /** The comm ID. */ + id: string; + /** Async-iterable for messages sent from backend. */ receiver: Channel; @@ -246,19 +249,19 @@ export interface RawComm { notify: (method: string, params?: Record) => boolean; /** Make a request to the backend comm. Resolves when backend responds. The tuple's - * first value indicates whether the comm was closed (and no request was emitted). + * first value indicates whether the comm was closed (and no request was emitted). * The second value is the result if the request was made. */ request: (method: string, params?: Record) => Promise<[boolean, any]>; /** Clear resources and sends `comm_close` to backend comm (unless the channel - * was closed by the backend already). */ + * was closed by the backend already). */ dispose: () => void; } /** * Communication channel. Dispose to close. */ -export interface Channel extends AsyncIterable, vscode.Disposable {} +export interface Channel extends AsyncIterable, vscode.Disposable { } /** Message from the backend. * From 991cd9ef0b5899bb8be2a6e9bdca79c44b994933 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Sun, 10 Aug 2025 05:19:25 -0400 Subject: [PATCH 17/48] Use camelCase --- extensions/positron-supervisor/src/KallichoreSession.ts | 2 +- extensions/positron-supervisor/src/RawComm.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 45cb06e25689..c3503034016c 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -758,7 +758,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { // If already deleted, it means a `comm_close` from the backend was // received and we don't need to send one. if (this._comms.delete(id)) { - comm.close_and_notify(); + comm.closeAndNotify(); } } }); diff --git a/extensions/positron-supervisor/src/RawComm.ts b/extensions/positron-supervisor/src/RawComm.ts index 2d67b788f50c..b8769ae0c116 100644 --- a/extensions/positron-supervisor/src/RawComm.ts +++ b/extensions/positron-supervisor/src/RawComm.ts @@ -67,7 +67,7 @@ export class RawCommImpl implements vscode.Disposable { this.closed = true; } - close_and_notify() { + closeAndNotify() { this.close(); const commClose = new CommCloseCommand(this.id); this.session.sendCommand(commClose); From 86e4bad5f3705e505c4d5bdf0107265470c3beeb Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Wed, 13 Aug 2025 11:31:26 +0200 Subject: [PATCH 18/48] Reformat utils --- extensions/positron-supervisor/src/util.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/extensions/positron-supervisor/src/util.ts b/extensions/positron-supervisor/src/util.ts index c91dac7f68c8..6da30ddb5162 100644 --- a/extensions/positron-supervisor/src/util.ts +++ b/extensions/positron-supervisor/src/util.ts @@ -123,8 +123,8 @@ type PayloadStructure = { /** * @description Type predicate to check if an object is VSBufferLike ({ buffer: Buffer }). - * @param {unknown} item - The item to check. - * @returns {boolean} True if the item is VSBufferLike, false otherwise. + * @param item - The item to check. + * @returns True if the item is VSBufferLike, false otherwise. */ function isVSBufferLike(item: unknown): item is VSBufferLike { return ( @@ -144,8 +144,8 @@ type PayloadWithDataValue = PayloadStructure & { /** * @description Type predicate to check if the payload has the required nested data.value structure. - * @param {unknown} payload - The payload to check. - * @returns {boolean} True if the payload has the expected structure, false otherwise. + * @param payload - The payload to check. + * @returns True if the payload has the expected structure, false otherwise. */ function isPayloadWithDataValue(payload: unknown): payload is PayloadWithDataValue { return ( @@ -164,9 +164,9 @@ function isPayloadWithDataValue(payload: unknown): payload is PayloadWithDataVal /** * @description Validates if an item is a Buffer or VSBufferLike and within the size limit. - * @param {unknown} item - The item to validate. - * @param {number} maxSize - The maximum allowed buffer size in bytes. - * @returns {Buffer | undefined} The Buffer instance if valid, otherwise undefined. + * @param item - The item to validate. + * @param maxSize - The maximum allowed buffer size in bytes. + * @returns The Buffer instance if valid, otherwise undefined. */ function validateAndGetBufferInstance(item: unknown, maxSize: number): Buffer | undefined { let bufferInstance: Buffer | undefined; @@ -195,8 +195,8 @@ function validateAndGetBufferInstance(item: unknown, maxSize: number): Buffer | * found in `payload.data.value.buffers`, converts them to base64 strings, * and restructures the content payload. If the expected structure isn't found, * the original payload is returned as content with empty buffers. - * @param {unknown} payload - The input payload, potentially containing serialized data and buffers. - * @returns {UnpackedResult} An object containing the processed content and an array of base64 buffer strings. + * @param payload - The input payload, potentially containing serialized data and buffers. + * @returns An object containing the processed content and an array of base64 buffer strings. * @export */ export function unpackSerializedObjectWithBuffers(payload: unknown): { From 0981470f61f5512f6adf510d5388d670ee52dc03 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 16 Jun 2025 17:44:41 +0200 Subject: [PATCH 19/48] Implement Ark DAP as raw comm --- .../src/client/positron-supervisor.d.ts | 115 +++++++++++++--- .../positron-r/src/positron-supervisor.d.ts | 127 ++++++++++++++--- extensions/positron-r/src/session.ts | 70 ++++++++-- .../src/positron-supervisor.d.ts | 130 ++++++++++++++++-- extensions/positron-supervisor/src/Channel.ts | 12 +- .../positron-supervisor/src/DapClient.ts | 85 ++++++++---- .../src/KallichoreAdapterApi.ts | 5 + .../src/KallichoreSession.ts | 91 +++++------- .../positron-supervisor/src/extension.ts | 9 +- .../src/positron-supervisor.d.ts | 69 ++++++++-- 10 files changed, 559 insertions(+), 154 deletions(-) diff --git a/extensions/positron-python/src/client/positron-supervisor.d.ts b/extensions/positron-python/src/client/positron-supervisor.d.ts index 7955deb3213e..e5e78624e2c2 100644 --- a/extensions/positron-python/src/client/positron-supervisor.d.ts +++ b/extensions/positron-python/src/client/positron-supervisor.d.ts @@ -34,6 +34,25 @@ export interface JupyterKernel { log(msg: string): void; } +/** + * Message sent from the frontend requesting a server to start + */ +export interface ServerStartMessage { + /** The IP address or host name on which the client is listening for server requests. The server is + * in charge of picking the exact port to communicate over, since the server is the + * one that binds to the port (to prevent race conditions). + */ + host: string; +} + +/** + * Message sent to the frontend to acknowledge that the corresponding server has started + */ +export interface ServerStartedMessage { + /** The port that the frontend should connect to on the `ip_address` it sent over */ + port: number; +} + /** * This set of type definitions defines the interfaces used by the Positron * Supervisor extension. @@ -50,7 +69,7 @@ export interface JupyterKernelSpec { argv: Array; /** The kernel's display name */ - display_name: string; // eslint-disable-line + display_name: string; // eslint-disable-line /** The language the kernel executes */ language: string; @@ -72,7 +91,7 @@ export interface JupyterKernelSpec { /** Function that starts the kernel given a JupyterSession object. * This is used to start the kernel if it's provided. In this case `argv` * is ignored. - */ + */ startKernel?: (session: JupyterSession, kernel: JupyterKernel) => Promise; } @@ -91,17 +110,6 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS */ startPositronLsp(clientId: string, ipAddress: string): Promise; - /** - * Convenience method for starting the Positron DAP server, if the - * language runtime supports it. - * - * @param clientId The ID of the client comm, created with - * `createPositronDapClientId()`. - * @param debugType Passed as `vscode.DebugConfiguration.type`. - * @param debugName Passed as `vscode.DebugConfiguration.name`. - */ - startPositronDap(clientId: string, debugType: string, debugName: string): Promise; - /** * Convenience method for creating a client id to pass to * `startPositronLsp()`. The caller can later remove the client using this @@ -110,11 +118,27 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS createPositronLspClientId(): string; /** - * Convenience method for creating a client id to pass to - * `startPositronDap()`. The caller can later remove the client using this - * id as well. + * Creates a server communication channel and returns both the comm and the port. + * + * @param targetName The name of the comm target + * @param host The IP address or host name for the server + * @returns A promise that resolves to a tuple of [RawComm, port number] + */ + createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; + + /** + * Start a raw comm for communication between frontend and backend. + * + * Unlike Positron clients, this kind of comm is private to the calling + * extension and its kernel. + * + * @param target_name Comm type, also used to generate comm identifier. + * @param params Optionally, additional parameters included in `comm_open`. */ - createPositronDapClientId(): string; + createComm( + target_name: string, + params?: Record, + ): Promise; /** * Method for emitting a message to the language server's Jupyter output @@ -129,7 +153,8 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * A Jupyter kernel is guaranteed to have a `showOutput()` * method, so we declare it non-optional. * - * @param channel The channel to show the output of. + * @param channel The name of the output channel to show. + * If not provided, the default channel is shown. */ showOutput(channel?: positron.LanguageRuntimeSessionChannel): void; @@ -157,6 +182,7 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * The Positron Supervisor API as exposed by the Positron Supervisor extension. */ export interface PositronSupervisorApi extends vscode.Disposable { + /** * Create a session for a Jupyter-compatible kernel. * @@ -210,3 +236,56 @@ export interface JupyterKernelExtra { init: (args: Array, delay: number) => void; }; } + +/** + * Raw comm unmanaged by Positron. + * + * This type of comm is not mapped to a Positron client. It lives entirely in + * the extension space and allows private communication between an extension and + * its kernel. + * + * It's a disposable. Dispose of it once it's closed or you're no longer using + * it. If the comm has not already been closed by the kernel, a client-initiated + * `comm_close` message is emitted. + */ +export interface RawComm { + /** The comm ID. */ + id: string; + + /** Async-iterable for messages sent from backend. */ + receiver: Channel; + + /** Send a notification to the backend comm. Returns `false` if comm was closed. */ + notify: (method: string, params?: Record) => boolean; + + /** Make a request to the backend comm. Resolves when backend responds. The tuple's + * first value indicates whether the comm was closed (and no request was emitted). + * The second value is the result if the request was made. */ + request: (method: string, params?: Record) => Promise<[boolean, any]>; + + /** Clear resources and sends `comm_close` to backend comm (unless the channel + * was closed by the backend already). */ + dispose: () => void; +} + +/** + * Communication channel. Dispose to close. + */ +export interface Channel extends AsyncIterable, vscode.Disposable { } + +/** Message from the backend. + * + * If a request, the `handle` method _must_ be called. + */ +export type CommBackendMessage = + | { + kind: 'request'; + method: string; + params?: Record; + handle: (handler: () => any) => void; + } + | { + kind: 'notification'; + method: string; + params?: Record; + }; diff --git a/extensions/positron-r/src/positron-supervisor.d.ts b/extensions/positron-r/src/positron-supervisor.d.ts index 142e801685fb..91f8248acd18 100644 --- a/extensions/positron-r/src/positron-supervisor.d.ts +++ b/extensions/positron-r/src/positron-supervisor.d.ts @@ -5,6 +5,7 @@ import * as vscode from 'vscode'; +// eslint-disable-next-line import/no-unresolved import * as positron from 'positron'; export interface JupyterSessionState { @@ -33,6 +34,25 @@ export interface JupyterKernel { log(msg: string): void; } +/** + * Message sent from the frontend requesting a server to start + */ +export interface ServerStartMessage { + /** The IP address or host name on which the client is listening for server requests. The server is + * in charge of picking the exact port to communicate over, since the server is the + * one that binds to the port (to prevent race conditions). + */ + host: string; +} + +/** + * Message sent to the frontend to acknowledge that the corresponding server has started + */ +export interface ServerStartedMessage { + /** The port that the frontend should connect to on the `ip_address` it sent over */ + port: number; +} + /** * This set of type definitions defines the interfaces used by the Positron * Supervisor extension. @@ -90,17 +110,6 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS */ startPositronLsp(clientId: string, ipAddress: string): Promise; - /** - * Convenience method for starting the Positron DAP server, if the - * language runtime supports it. - * - * @param clientId The ID of the client comm, created with - * `createPositronDapClientId()`. - * @param debugType Passed as `vscode.DebugConfiguration.type`. - * @param debugName Passed as `vscode.DebugConfiguration.name`. - */ - startPositronDap(clientId: string, debugType: string, debugName: string): Promise; - /** * Convenience method for creating a client id to pass to * `startPositronLsp()`. The caller can later remove the client using this @@ -109,11 +118,27 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS createPositronLspClientId(): string; /** - * Convenience method for creating a client id to pass to - * `startPositronDap()`. The caller can later remove the client using this - * id as well. + * Creates a server communication channel and returns both the comm and the port. + * + * @param targetName The name of the comm target + * @param host The IP address or host name for the server + * @returns A promise that resolves to a tuple of [RawComm, port number] + */ + createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; + + /** + * Start a raw comm for communication between frontend and backend. + * + * Unlike Positron clients, this kind of comm is private to the calling + * extension and its kernel. + * + * @param target_name Comm type, also used to generate comm identifier. + * @param params Optionally, additional parameters included in `comm_open`. */ - createPositronDapClientId(): string; + createComm( + target_name: string, + params?: Record, + ): Promise; /** * Method for emitting a message to the language server's Jupyter output @@ -128,7 +153,8 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * A Jupyter kernel is guaranteed to have a `showOutput()` * method, so we declare it non-optional. * - * @param channel The channel to show the output of. + * @param channel The name of the output channel to show. + * If not provided, the default channel is shown. */ showOutput(channel?: positron.LanguageRuntimeSessionChannel): void; @@ -210,3 +236,72 @@ export interface JupyterKernelExtra { init: (args: Array, delay: number) => void; }; } + +/** + * Raw comm unmanaged by Positron. + * + * This type of comm is not mapped to a Positron client. It lives entirely in + * the extension space and allows private communication between an extension and + * its kernel. + * + * It's a disposable. Dispose of it once it's closed or you're no longer using + * it. If the comm has not already been closed by the kernel, a client-initiated + * `comm_close` message is emitted. + */ +export interface RawComm { + /** The comm ID. */ + id: string; + + /** Async-iterable for messages sent from backend. */ + receiver: Channel; + + /** Send a notification to the backend comm. Returns `false` if comm was closed. */ + notify: (method: string, params?: Record) => boolean; + + /** Make a request to the backend comm. Resolves when backend responds. The tuple's + * first value indicates whether the comm was closed (and no request was emitted). + * The second value is the result if the request was made. */ + request: (method: string, params?: Record) => Promise<[boolean, any]>; + + /** Clear resources and sends `comm_close` to backend comm (unless the channel + * was closed by the backend already). */ + dispose: () => void; +} + +/** + * Communication channel. Dispose to close. + */ +export interface Channel extends AsyncIterable, vscode.Disposable { } + +/** Message from the backend. + * + * If a request, the `handle` method _must_ be called. + */ +export type CommBackendMessage = + | { + kind: 'request'; + method: string; + params?: Record; + handle: (handler: () => any) => void; + } + | { + kind: 'notification'; + method: string; + params?: Record; + }; + +/** + * Interface for DAP communication instances + */ +export interface DapCommInterface { + readonly targetName: string; + readonly debugType: string; + readonly debugName: string; + + readonly comm?: RawComm; + readonly serverPort?: number; + + createComm(): Promise; + handleMessage(msg: any): boolean; + dispose(): void; +} diff --git a/extensions/positron-r/src/session.ts b/extensions/positron-r/src/session.ts index 6205329a6fc9..43cc7954ae2a 100644 --- a/extensions/positron-r/src/session.ts +++ b/extensions/positron-r/src/session.ts @@ -7,9 +7,9 @@ import * as positron from 'positron'; import * as vscode from 'vscode'; import PQueue from 'p-queue'; -import { PositronSupervisorApi, JupyterKernelSpec, JupyterLanguageRuntimeSession, JupyterKernelExtra } from './positron-supervisor'; +import { PositronSupervisorApi, JupyterKernelSpec, JupyterLanguageRuntimeSession, JupyterKernelExtra, RawComm, DapCommInterface } from './positron-supervisor'; import { ArkLsp, ArkLspState } from './lsp'; -import { delay, whenTimeout, timeout } from './util'; +import { delay, whenTimeout, timeout, PromiseHandles } from './util'; import { ArkAttachOnStartup, ArkDelayStartup } from './startup'; import { RHtmlWidget, getResourceRoots } from './htmlwidgets'; import { randomUUID } from 'crypto'; @@ -42,7 +42,6 @@ interface Locale { * Protocol client. */ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposable { - /** The Language Server Protocol client wrapper */ private _lsp: ArkLsp; @@ -62,6 +61,9 @@ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposa /** The Jupyter kernel-based session implementing the Language Runtime API */ private _kernel?: JupyterLanguageRuntimeSession; + /** The DAP communication channel */ + private _dapComm?: DapCommInterface; + /** The emitter for language runtime messages */ private _messageEmitter = new vscode.EventEmitter(); @@ -290,7 +292,8 @@ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposa this.onConsoleWidthChange(newWidth); }); } - return this._kernel.start(); + + return await this._kernel.start(); } private async onConsoleWidthChange(newWidth: number): Promise { @@ -832,21 +835,68 @@ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposa private async startDap(): Promise { if (this._kernel) { try { - let clientId = this._kernel.createPositronDapClientId(); - await this._kernel.startPositronDap(clientId, 'ark', 'Ark Positron R'); + // Get the supervisor extension API + const supervisorExt = vscode.extensions.getExtension('positron.positron-supervisor'); + if (!supervisorExt) { + throw new Error('Positron Supervisor extension not found'); + } + + const supervisorApi = await supervisorExt.activate(); + if (!supervisorApi.implementations) { + throw new Error('Supervisor API implementations not available'); + } + const dapComm = supervisorApi.implementations.DapComm; + if (!dapComm) { + throw new Error('DapComm implementation not available'); + } + + this._dapComm = new dapComm(this._kernel!, 'ark_dap', 'ark', 'Ark Positron R'); + await this._dapComm!.createComm(); + this.startDapMessageLoop(); } catch (err) { - this._kernel.emitJupyterLog(`Error starting DAP: ${err}`, vscode.LogLevel.Error); + LOGGER.error(`Error starting DAP: ${err}`); } } } + /** + * Handle DAP messages in an infinite loop + */ + private async startDapMessageLoop(): Promise { + LOGGER.info('Starting DAP loop'); + + if (!this._dapComm?.comm) { + throw new Error('Must create comm before use'); + } + + for await (const message of this._dapComm.comm.receiver) { + LOGGER.trace('Received DAP message:', JSON.stringify(message)); + + if (!this._dapComm.handleMessage(message)) { + LOGGER.info(`Unknown DAP message: ${message.method}`); + + if (message.kind === 'request') { + message.handle(() => { throw new Error(`Unknown request '${message.method}' for DAP comm`) }); + } + } + } + + LOGGER.info('Exiting DAP loop'); + this._dapComm?.dispose(); + } + private async onStateChange(state: positron.RuntimeState): Promise { this._state = state; if (state === positron.RuntimeState.Ready) { - await this.startDap(); - await this.setConsoleWidth(); + await Promise.all([ + this.startDap(), + this.setConsoleWidth() + ]); } else if (state === positron.RuntimeState.Exited) { - await this.deactivateLsp('session exited'); + await Promise.all([ + this._dapComm?.dispose(), + this.deactivateLsp('session exited'), + ]); } } diff --git a/extensions/positron-reticulate/src/positron-supervisor.d.ts b/extensions/positron-reticulate/src/positron-supervisor.d.ts index ff95ec33d0c8..95f61db2d29c 100644 --- a/extensions/positron-reticulate/src/positron-supervisor.d.ts +++ b/extensions/positron-reticulate/src/positron-supervisor.d.ts @@ -34,9 +34,28 @@ export interface JupyterKernel { log(msg: string): void; } +/** + * Message sent from the frontend requesting a server to start + */ +export interface ServerStartMessage { + /** The IP address or host name on which the client is listening for server requests. The server is + * in charge of picking the exact port to communicate over, since the server is the + * one that binds to the port (to prevent race conditions). + */ + host: string; +} + +/** + * Message sent to the frontend to acknowledge that the corresponding server has started + */ +export interface ServerStartedMessage { + /** The port that the frontend should connect to on the `ip_address` it sent over */ + port: number; +} + /** * This set of type definitions defines the interfaces used by the Positron - * Positron Supervisor extension. + * Supervisor extension. */ /** @@ -84,19 +103,42 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * Convenience method for starting the Positron LSP server, if the * language runtime supports it. * + * @param clientId The ID of the client comm, created with + * `createPositronLspClientId()`. * @param ipAddress The address of the client that will connect to the * language server. */ - startPositronLsp(ipAddress: string): Promise; + startPositronLsp(clientId: string, ipAddress: string): Promise; /** - * Convenience method for starting the Positron DAP server, if the - * language runtime supports it. + * Convenience method for creating a client id to pass to + * `startPositronLsp()`. The caller can later remove the client using this + * id as well. + */ + createPositronLspClientId(): string; + + /** + * Creates a server communication channel and returns both the comm and the port. + * + * @param targetName The name of the comm target + * @param host The IP address or host name for the server + * @returns A promise that resolves to a tuple of [RawComm, port number] + */ + createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; + + /** + * Start a raw comm for communication between frontend and backend. + * + * Unlike Positron clients, this kind of comm is private to the calling + * extension and its kernel. * - * @param debugType Passed as `vscode.DebugConfiguration.type`. - * @param debugName Passed as `vscode.DebugConfiguration.name`. + * @param target_name Comm type, also used to generate comm identifier. + * @param params Optionally, additional parameters included in `comm_open`. */ - startPositronDap(debugType: string, debugName: string): Promise; + createComm( + target_name: string, + params?: Record, + ): Promise; /** * Method for emitting a message to the language server's Jupyter output @@ -110,8 +152,18 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS /** * A Jupyter kernel is guaranteed to have a `showOutput()` * method, so we declare it non-optional. + * + * @param channel The name of the output channel to show. + * If not provided, the default channel is shown. + */ + showOutput(channel?: positron.LanguageRuntimeSessionChannel): void; + + /** + * Return a list of output channels + * + * @returns A list of output channels available on this runtime */ - showOutput(): void; + listOutputChannels(): positron.LanguageRuntimeSessionChannel[]; /** * A Jupyter kernel is guaranteed to have a `callMethod()` method; it uses @@ -152,18 +204,25 @@ export interface PositronSupervisorApi extends vscode.Disposable { extra?: JupyterKernelExtra | undefined, ): Promise; + /** + * Validate an existing session for a Jupyter-compatible kernel. + */ + validateSession(sessionId: string): Promise; + /** * Restore a session for a Jupyter-compatible kernel. * * @param runtimeMetadata The metadata for the language runtime to be * wrapped by the adapter. * @param sessionMetadata The metadata for the session to be reconnected. + * @param dynState The initial dynamic state of the session. * * @returns A JupyterLanguageRuntimeSession that wraps the kernel. */ restoreSession( runtimeMetadata: positron.LanguageRuntimeMetadata, - sessionMetadata: positron.RuntimeSessionMetadata + sessionMetadata: positron.RuntimeSessionMetadata, + dynState: positron.LanguageRuntimeDynState, ): Promise; } @@ -177,3 +236,56 @@ export interface JupyterKernelExtra { init: (args: Array, delay: number) => void; }; } + +/** + * Raw comm unmanaged by Positron. + * + * This type of comm is not mapped to a Positron client. It lives entirely in + * the extension space and allows private communication between an extension and + * its kernel. + * + * It's a disposable. Dispose of it once it's closed or you're no longer using + * it. If the comm has not already been closed by the kernel, a client-initiated + * `comm_close` message is emitted. + */ +export interface RawComm { + /** The comm ID. */ + id: string; + + /** Async-iterable for messages sent from backend. */ + receiver: Channel; + + /** Send a notification to the backend comm. Returns `false` if comm was closed. */ + notify: (method: string, params?: Record) => boolean; + + /** Make a request to the backend comm. Resolves when backend responds. The tuple's + * first value indicates whether the comm was closed (and no request was emitted). + * The second value is the result if the request was made. */ + request: (method: string, params?: Record) => Promise<[boolean, any]>; + + /** Clear resources and sends `comm_close` to backend comm (unless the channel + * was closed by the backend already). */ + dispose: () => void; +} + +/** + * Communication channel. Dispose to close. + */ +export interface Channel extends AsyncIterable, vscode.Disposable { } + +/** Message from the backend. + * + * If a request, the `handle` method _must_ be called. + */ +export type CommBackendMessage = + | { + kind: 'request'; + method: string; + params?: Record; + handle: (handler: () => any) => void; + } + | { + kind: 'notification'; + method: string; + params?: Record; + }; diff --git a/extensions/positron-supervisor/src/Channel.ts b/extensions/positron-supervisor/src/Channel.ts index b92b56268da6..fb77b538220d 100644 --- a/extensions/positron-supervisor/src/Channel.ts +++ b/extensions/positron-supervisor/src/Channel.ts @@ -26,7 +26,7 @@ export function channel(): [Sender, Receiver] { export class Sender implements vscode.Disposable { private disposables: vscode.Disposable[] = []; - constructor(private state: ChannelState) {} + constructor(private state: ChannelState) { } /** * Sends a value to the channel. @@ -40,7 +40,8 @@ export class Sender implements vscode.Disposable { if (this.state.pending_consumers.length > 0) { // There is a consumer waiting, resolve it immediately - this.state.pending_consumers.shift()!({ value, done: false }); + const consumer = this.state.pending_consumers.shift(); + consumer!({ value, done: false }); } else { // No consumer waiting, queue up the value this.state.queue.push(value); @@ -68,7 +69,7 @@ export class Receiver implements AsyncIterable, AsyncIterator, vscode.D private i = 0; private disposables: vscode.Disposable[] = []; - constructor(private state: ChannelState) {} + constructor(private state: ChannelState) { } [Symbol.asyncIterator]() { return this; @@ -78,6 +79,9 @@ export class Receiver implements AsyncIterable, AsyncIterator, vscode.D if (this.state.queue.length > 0) { ++this.i; + // Get the value from queue first, before any potential await + const value = this.state.queue.shift()!; + // Yield regularly to event loop to avoid starvation. Sends are // synchronous and handlers might be synchronous as well. if (this.i > YIELD_THRESHOLD) { @@ -85,7 +89,7 @@ export class Receiver implements AsyncIterable, AsyncIterator, vscode.D await delay(0); } - return { value: this.state.queue.shift()!, done: false }; + return { value, done: false }; } // If nothing in the queue and the channel is closed, we're done diff --git a/extensions/positron-supervisor/src/DapClient.ts b/extensions/positron-supervisor/src/DapClient.ts index 731b16119947..a4199e9c10c4 100644 --- a/extensions/positron-supervisor/src/DapClient.ts +++ b/extensions/positron-supervisor/src/DapClient.ts @@ -5,68 +5,107 @@ import * as vscode from 'vscode'; import * as positron from 'positron'; -import { JupyterLanguageRuntimeSession } from './positron-supervisor'; +import { JupyterLanguageRuntimeSession, RawComm } from './positron-supervisor'; /** * A Debug Adapter Protocol (DAP) client instance; handles messages from the * kernel side of the DAP and forwards them to the debug adapter. */ -export class DapClient { +export class DapComm { + public get comm(): RawComm | undefined { + return this._comm; + } + public get port(): number | undefined { + return this._port; + } + + private _comm?: RawComm; + private _port?: number; + /** Message counter; used for creating unique message IDs */ - private static _counter = 0; + private messageCounter = 0; - private _msgStem: string; + /** Random stem for messages */ + private msgStem: string; - constructor(readonly clientId: string, - readonly serverPort: number, + constructor( + private session: JupyterLanguageRuntimeSession, + readonly targetName: string, readonly debugType: string, readonly debugName: string, - readonly session: JupyterLanguageRuntimeSession) { + ) { // Generate 8 random hex characters for the message stem - this._msgStem = Math.random().toString(16).slice(2, 10); + this.msgStem = Math.random().toString(16).slice(2, 10); + } + + async createComm(): Promise { + // NOTE: Ideally we'd allow connecting to any network interface but the + // `debugServer` property passed in the configuration below needs to be + // localhost. + const host = '127.0.0.1'; + + const [comm, serverPort] = await this.session.createServerComm(this.targetName, host); + + this._comm = comm; + this._port = serverPort; } - handleDapMessage(msg: any) { - switch (msg.msg_type) { + handleMessage(msg: any): boolean { + if (msg.kind === 'request') { + return false; + } + + switch (msg.method) { // The runtime is in control of when to start a debug session. // When this happens, we attach automatically to the runtime // with a synthetic configuration. case 'start_debug': { - this.session.emitJupyterLog(`Starting debug session for DAP server ${this.clientId}`); + this.session.emitJupyterLog(`Starting debug session for DAP server ${this.comm!.id}`); const config: vscode.DebugConfiguration = { type: this.debugType, name: this.debugName, request: 'attach', - debugServer: this.serverPort, + debugServer: this.port, internalConsoleOptions: 'neverOpen', }; vscode.debug.startDebugging(undefined, config); - break; + + return true; } // If the DAP has commands to execute, such as "n", "f", or "Q", // it sends events to let us do it from here. case 'execute': { - this.session.execute( - msg.content.command, - this._msgStem + '-dap-' + DapClient._counter++, - positron.RuntimeCodeExecutionMode.Interactive, - positron.RuntimeErrorBehavior.Stop - ); - break; + const command = msg.params?.command; + if (command) { + this.session.execute( + command, + this.msgStem + '-dap-' + this.messageCounter++, + positron.RuntimeCodeExecutionMode.Interactive, + positron.RuntimeErrorBehavior.Stop + ); + } + + return true; } // We use the restart button as a shortcut for restarting the runtime case 'restart': { this.session.restart(); - break; + return true; } default: { - this.session.emitJupyterLog(`Unknown DAP command: ${msg.msg_type}`); - break; + return false; } } } + + /** + * Dispose of the underlying comm, if present. + */ + dispose(): void { + this._comm?.dispose(); + } } diff --git a/extensions/positron-supervisor/src/KallichoreAdapterApi.ts b/extensions/positron-supervisor/src/KallichoreAdapterApi.ts index 4301907b9850..1aefe64dba70 100644 --- a/extensions/positron-supervisor/src/KallichoreAdapterApi.ts +++ b/extensions/positron-supervisor/src/KallichoreAdapterApi.ts @@ -119,7 +119,12 @@ function constructWebSocketUri(apiBasePath: string, sessionId: string): string { return `ws://${uri.authority}/sessions/${sessionId}/channels`; } +import { DapComm } from './DapClient'; + export class KCApi implements PositronSupervisorApi { + readonly implementations = { + DapComm + }; /** The instance of the API; the API is code-generated from the Kallichore * OpenAPI spec */ diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index c3503034016c..4e9168ecc7e5 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -8,7 +8,7 @@ import * as positron from 'positron'; import * as os from 'os'; import * as path from 'path'; import * as fs from 'fs'; -import { CommBackendMessage, JupyterKernelExtra, JupyterKernelSpec, JupyterLanguageRuntimeSession, JupyterSession, RawComm } from './positron-supervisor'; +import { CommBackendMessage, JupyterKernelExtra, JupyterKernelSpec, JupyterLanguageRuntimeSession, JupyterSession, RawComm, ServerStartMessage, ServerStartedMessage } from './positron-supervisor'; import { ActiveSession, ConnectionInfo, DefaultApi, HttpError, InterruptMode, NewSession, RestartSession, Status, VarAction, VarActionType } from './kcclient/api'; import { JupyterMessage } from './jupyter/JupyterMessage'; import { JupyterRequest } from './jupyter/JupyterRequest'; @@ -33,7 +33,6 @@ import { RpcReplyCommand } from './jupyter/RpcReplyCommand'; import { JupyterCommRequest } from './jupyter/JupyterCommRequest'; import { Comm } from './Comm'; import { CommMsgRequest } from './jupyter/CommMsgRequest'; -import { DapClient } from './DapClient'; import { SocketSession } from './ws/SocketSession'; import { KernelOutputMessage } from './ws/KernelMessage'; import { UICommRequest } from './UICommRequest'; @@ -126,9 +125,6 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { /** Whether it is possible to connect to the session's websocket */ private _canConnect = true; - /** The Debug Adapter Protocol client, if any */ - private _dapClient: DapClient | undefined; - /** A map of pending comm startups */ private _startingComms: Map> = new Map(); @@ -462,58 +458,45 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { return startPromise.promise; } + createPositronLspClientId(): string { + return `positron-lsp-${this.runtimeMetadata.languageId}-${createUniqueId()}`; + } + /** - * Requests that the kernel start a Debug Adapter Protocol server, and - * connect it to the client locally on the given TCP address. + * Creates a server communication channel and returns both the comm and the port. * - * @param clientId The ID of the client comm, created with - * `createPositronDapClientId()`. - * @param debugType Passed as `vscode.DebugConfiguration.type`. - * @param debugName Passed as `vscode.DebugConfiguration.name`. + * @param target_name The name of the comm target + * @param host The IP address or host name for the server + * @returns A promise that resolves to a tuple of [RawComm, port number] */ - async startPositronDap(clientId: string, debugType: string, debugName: string) { - // NOTE: Ideally we'd connect to any address but the - // `debugServer` property passed in the configuration below - // needs to be localhost. - const ipAddress = '127.0.0.1'; - - // TODO: Should we query the kernel to see if it can create a DAP - // (QueryInterface style) instead of just demanding it? - // - // The Jupyter kernel spec does not provide a way to query for - // supported comms; the only way to know is to try to create one. + async createServerComm(target_name: string, host: string): Promise<[RawComm, number]> { + this.log(`Starting server comm '${target_name}' for ${host}`); + const comm = await this.createComm(target_name, { ip_address: host }); - this.log(`Starting DAP server ${clientId} for ${ipAddress}`, vscode.LogLevel.Debug); + const result = await comm.receiver.next(); - // Notify Positron that we're handling messages from this client - this._disposables.push(positron.runtime.registerClientInstance(clientId)); + if (result.done) { + comm.dispose(); + throw new Error('Comm was closed before sending a `server_started` message'); + } - await this.createClient( - clientId, - positron.RuntimeClientType.Dap, - { ip_address: ipAddress } - ); + const message = result.value; - // Create a promise that will resolve when the DAP starts on the server - // side. When the promise resolves we obtain the port the client should - // connect on. - const startPromise = new PromiseHandles(); - this._startingComms.set(clientId, startPromise); - - // Immediately await that promise because `startPositronDap()` handles the full - // DAP setup, unlike the LSP where the extension finishes the setup. - const port = await startPromise.promise; + if (message.method !== 'server_started') { + comm.dispose(); + throw new Error('Comm was closed before sending a `server_started` message'); + } - // Create the DAP client message handler - this._dapClient = new DapClient(clientId, port, debugType, debugName, this); - } + const serverStarted = message.params as any; + const port = serverStarted.port; - createPositronLspClientId(): string { - return `positron-lsp-${this.runtimeMetadata.languageId}-${createUniqueId()}`; - } + if (typeof port !== 'number') { + comm.dispose(); + throw new Error('`server_started` message doesn\'t include a port'); + } - createPositronDapClientId(): string { - return `positron-dap-${this.runtimeMetadata.languageId}-${createUniqueId()}`; + this.log(`Started server comm '${target_name}' for ${host} on port ${port}`); + return [comm, port]; } /** @@ -1979,25 +1962,17 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { } else { tx.send({ kind: 'notification', method: rpcMsg.method, params: rpcMsg.params }); } + return; } + break; } } - // TODO: Make DAP and LSP comms unmanaged + // TODO: Make LSP comms unmanaged if (msg.header.msg_type === 'comm_msg') { const commMsg = msg.content as JupyterCommMsg; - - // If we have a DAP client active and this is a comm message intended - // for that client, forward the message. - if (this._dapClient) { - const comm = this._clients.get(commMsg.comm_id); - if (comm && comm.id === this._dapClient.clientId) { - this._dapClient.handleDapMessage(commMsg.data); - } - } - // If this is a `server_started` message, resolve the promise that // was created when the comm was started. if (commMsg.data.msg_type === 'server_started') { diff --git a/extensions/positron-supervisor/src/extension.ts b/extensions/positron-supervisor/src/extension.ts index e9ea00e8e11f..e051e3afd59d 100644 --- a/extensions/positron-supervisor/src/extension.ts +++ b/extensions/positron-supervisor/src/extension.ts @@ -8,6 +8,7 @@ import * as positron from 'positron'; import { PositronSupervisorApi } from './positron-supervisor'; import { KCApi } from './KallichoreAdapterApi'; +import { DapComm } from './DapClient'; /** Singleton instance of the Kallichore API wrapper */ export let API_INSTANCE: KCApi; @@ -24,7 +25,13 @@ export function activate(context: vscode.ExtensionContext): PositronSupervisorAp log.show(); })); - return API_INSTANCE; + // Create extended API that includes implementations + const extendedApi = Object.create(API_INSTANCE); + extendedApi.implementations = { + DapComm + }; + + return extendedApi; } export function deactivate() { diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index e839a5906aa2..87e06b82c681 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -34,6 +34,25 @@ export interface JupyterKernel { log(msg: string): void; } +/** + * Message sent from the frontend requesting a server to start + */ +export interface ServerStartMessage { + /** The IP address or host name on which the client is listening for server requests. The server is + * in charge of picking the exact port to communicate over, since the server is the + * one that binds to the port (to prevent race conditions). + */ + host: string; +} + +/** + * Message sent to the frontend to acknowledge that the corresponding server has started + */ +export interface ServerStartedMessage { + /** The port that the frontend should connect to on the `ip_address` it sent over */ + port: number; +} + /** * This set of type definitions defines the interfaces used by the Positron * Supervisor extension. @@ -91,17 +110,6 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS */ startPositronLsp(clientId: string, ipAddress: string): Promise; - /** - * Convenience method for starting the Positron DAP server, if the - * language runtime supports it. - * - * @param clientId The ID of the client comm, created with - * `createPositronDapClientId()`. - * @param debugType Passed as `vscode.DebugConfiguration.type`. - * @param debugName Passed as `vscode.DebugConfiguration.name`. - */ - startPositronDap(clientId: string, debugType: string, debugName: string): Promise; - /** * Convenience method for creating a client id to pass to * `startPositronLsp()`. The caller can later remove the client using this @@ -110,11 +118,13 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS createPositronLspClientId(): string; /** - * Convenience method for creating a client id to pass to - * `startPositronDap()`. The caller can later remove the client using this - * id as well. + * Creates a server communication channel and returns both the comm and the port. + * + * @param targetName The name of the comm target + * @param host The IP address or host name for the server + * @returns A promise that resolves to a tuple of [RawComm, port number] */ - createPositronDapClientId(): string; + createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; /** * Start a raw comm for communication between frontend and backend. @@ -214,6 +224,13 @@ export interface PositronSupervisorApi extends vscode.Disposable { sessionMetadata: positron.RuntimeSessionMetadata, dynState: positron.LanguageRuntimeDynState, ): Promise; + + /** + * Implementation classes exposed for use by other extensions + */ + readonly implementations: { + DapComm: DapCommConstructor; + }; } /** Specific functionality implemented by runtimes */ @@ -279,3 +296,25 @@ export type CommBackendMessage = method: string; params?: Record; }; + +export interface DapCommInterface { + readonly targetName: string; + readonly debugType: string; + readonly debugName: string; + + readonly comm?: RawComm; + readonly serverPort?: number; + + createComm(): Promise; + handleMessage(msg: any): boolean; + dispose(): void; +} + +export interface DapCommConstructor { + new( + session: JupyterLanguageRuntimeSession, + targetName: string, + debugType: string, + debugName: string, + ): DapCommInterface; +} From 9730abf7b577db50e61062d3918962437d927f03 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 19 Aug 2025 09:33:47 +0200 Subject: [PATCH 20/48] Don't list unmanaged comms as clients --- .../src/KallichoreSession.ts | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 4e9168ecc7e5..49fbfa80324f 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -150,7 +150,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { private readonly _clients: Map = new Map(); /** A map of active comms unmanaged by Positron */ - private readonly _comms: Map]> = new Map(); + private readonly _rawComms: Map]> = new Map(); /** The kernel's log file, if any. */ private _kernelLogFile: string | undefined; @@ -733,14 +733,14 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { const [tx, rx] = channel(); const comm = new RawCommImpl(id, this, rx); - this._comms.set(id, [comm, tx]); + this._rawComms.set(id, [comm, tx]); // Disposal handler that allows extension to initiate close comm comm.register({ dispose: () => { // If already deleted, it means a `comm_close` from the backend was // received and we don't need to send one. - if (this._comms.delete(id)) { + if (this._rawComms.delete(id)) { comm.closeAndNotify(); } } @@ -815,11 +815,17 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { const comms = reply.comms; // Unwrap the comm info and add it to the result for (const key in comms) { + // Don't list as client if this is an unmanaged comm + if (this._rawComms.has(key)) { + continue; + } + if (comms.hasOwnProperty(key)) { const target = comms[key].target_name; result[key] = target; // If we don't have a comm object for this comm, create one if (!this._clients.has(key)) { + // Should this be renamed to Client? this._clients.set(key, new Comm(key, target)); } @@ -1778,12 +1784,12 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { this._clients.clear(); // Close all raw comms - for (const [comm, tx] of this._comms.values()) { + for (const [comm, tx] of this._rawComms.values()) { // Don't dispose of comm, this resource is owned by caller of `createComm()`. (comm as RawCommImpl).close(); tx.dispose(); } - this._comms.clear(); + this._rawComms.clear(); // Clear any starting comms this._startingComms.forEach((promise) => { @@ -1903,7 +1909,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { // promise. if (msg.header.msg_type === 'comm_msg') { const commMsg = msg.content as JupyterCommMsg; - if (this._comms.has(commMsg.comm_id)) { + if (this._rawComms.has(commMsg.comm_id)) { return; } } @@ -1935,12 +1941,12 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { switch (msg.header.msg_type) { case 'comm_close': { const closeMsg = msg.content as JupyterCommClose; - const commHandle = this._comms.get(closeMsg.comm_id); + const commHandle = this._rawComms.get(closeMsg.comm_id); if (commHandle) { // Delete first, this prevents the channel disposable from sending a // `comm_close` back - this._comms.delete(closeMsg.comm_id); + this._rawComms.delete(closeMsg.comm_id); const [comm, _] = commHandle; comm.dispose(); @@ -1951,7 +1957,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { case 'comm_msg': { const commMsg = msg.content as JupyterCommMsg; - const commHandle = this._comms.get(commMsg.comm_id); + const commHandle = this._rawComms.get(commMsg.comm_id); if (commHandle) { const [_, tx] = commHandle; From d90a7e755e835a2927e12f06c165d48cc37f812b Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 19 Aug 2025 09:35:01 +0200 Subject: [PATCH 21/48] Remove `positron.dap` from client types --- extensions/positron-supervisor/src/KallichoreSession.ts | 4 ++-- extensions/positron-zed/src/positronZedLanguageRuntime.ts | 1 - src/positron-dts/positron.d.ts | 1 - src/vs/workbench/api/common/positron/extHostTypes.positron.ts | 1 - 4 files changed, 2 insertions(+), 5 deletions(-) diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 49fbfa80324f..0ae5a5690305 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -776,7 +776,6 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { // client-initiated creation if (type === positron.RuntimeClientType.Variables || type === positron.RuntimeClientType.Lsp || - type === positron.RuntimeClientType.Dap || type === positron.RuntimeClientType.Ui || type === positron.RuntimeClientType.Help || type === positron.RuntimeClientType.IPyWidgetControl) { @@ -1952,6 +1951,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { comm.dispose(); return; } + break; } @@ -1976,7 +1976,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { } } - // TODO: Make LSP comms unmanaged + // TODO: Make LSP comms unmanaged and remove this branch if (msg.header.msg_type === 'comm_msg') { const commMsg = msg.content as JupyterCommMsg; // If this is a `server_started` message, resolve the promise that diff --git a/extensions/positron-zed/src/positronZedLanguageRuntime.ts b/extensions/positron-zed/src/positronZedLanguageRuntime.ts index eb69a8f84d62..0249e6344da1 100644 --- a/extensions/positron-zed/src/positronZedLanguageRuntime.ts +++ b/extensions/positron-zed/src/positronZedLanguageRuntime.ts @@ -957,7 +957,6 @@ export class PositronZedRuntimeSession implements positron.LanguageRuntimeSessio case positron.RuntimeClientType.Help: case positron.RuntimeClientType.Lsp: - case positron.RuntimeClientType.Dap: // These types aren't currently supported by Zed, so close the // comm immediately to signal this to the client. this._onDidReceiveRuntimeMessage.fire({ diff --git a/src/positron-dts/positron.d.ts b/src/positron-dts/positron.d.ts index f5d74ed5747c..efae57d0d977 100644 --- a/src/positron-dts/positron.d.ts +++ b/src/positron-dts/positron.d.ts @@ -686,7 +686,6 @@ declare module 'positron' { export enum RuntimeClientType { Variables = 'positron.variables', Lsp = 'positron.lsp', - Dap = 'positron.dap', Plot = 'positron.plot', DataExplorer = 'positron.dataExplorer', Ui = 'positron.ui', diff --git a/src/vs/workbench/api/common/positron/extHostTypes.positron.ts b/src/vs/workbench/api/common/positron/extHostTypes.positron.ts index 8ccb4c46135e..2236831cda9e 100644 --- a/src/vs/workbench/api/common/positron/extHostTypes.positron.ts +++ b/src/vs/workbench/api/common/positron/extHostTypes.positron.ts @@ -185,7 +185,6 @@ export enum PositronChatMode { export enum RuntimeClientType { Variables = 'positron.variables', Lsp = 'positron.lsp', - Dap = 'positron.dap', Plot = 'positron.plot', DataExplorer = 'positron.dataExplorer', Ui = 'positron.ui', From 883fd68303c73a026ba6a055a5bd330277818b4c Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 19 Aug 2025 09:52:45 +0200 Subject: [PATCH 22/48] Log `start_debug` errors --- extensions/positron-supervisor/src/DapClient.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/extensions/positron-supervisor/src/DapClient.ts b/extensions/positron-supervisor/src/DapClient.ts index a4199e9c10c4..f3d5613ccdc8 100644 --- a/extensions/positron-supervisor/src/DapClient.ts +++ b/extensions/positron-supervisor/src/DapClient.ts @@ -69,7 +69,18 @@ export class DapComm { debugServer: this.port, internalConsoleOptions: 'neverOpen', }; - vscode.debug.startDebugging(undefined, config); + + // Log errors because this sometimes fail at + // https://github.com/posit-dev/positron/blob/71686862/src/vs/workbench/contrib/debug/browser/debugService.ts#L361 + // because `hasDebugged` is undefined. + try { + vscode.debug.startDebugging(undefined, config); + } catch (err) { + this.session.emitJupyterLog( + `Can't start debug session for DAP server ${this.comm!.id}: ${err}`, + vscode.LogLevel.Warning + ); + } return true; } From de2a82ab63f39a22ffad8c20eb3fe424596f259b Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 19 Aug 2025 09:54:51 +0200 Subject: [PATCH 23/48] Rename `DapClient.ts` to `DapComm.ts` --- .../positron-supervisor/src/{DapClient.ts => DapComm.ts} | 0 extensions/positron-supervisor/src/KallichoreAdapterApi.ts | 4 ++-- extensions/positron-supervisor/src/extension.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename extensions/positron-supervisor/src/{DapClient.ts => DapComm.ts} (100%) diff --git a/extensions/positron-supervisor/src/DapClient.ts b/extensions/positron-supervisor/src/DapComm.ts similarity index 100% rename from extensions/positron-supervisor/src/DapClient.ts rename to extensions/positron-supervisor/src/DapComm.ts diff --git a/extensions/positron-supervisor/src/KallichoreAdapterApi.ts b/extensions/positron-supervisor/src/KallichoreAdapterApi.ts index 1aefe64dba70..644c4691feaa 100644 --- a/extensions/positron-supervisor/src/KallichoreAdapterApi.ts +++ b/extensions/positron-supervisor/src/KallichoreAdapterApi.ts @@ -15,6 +15,8 @@ import { Barrier, PromiseHandles, withTimeout } from './async'; import { LogStreamer } from './LogStreamer'; import { createUniqueId, summarizeError, summarizeHttpError } from './util'; import { namedPipeInterceptor } from './NamedPipeHttpAgent'; +import { DapComm } from './DapComm'; + const KALLICHORE_STATE_KEY = 'positron-supervisor.v2'; @@ -119,8 +121,6 @@ function constructWebSocketUri(apiBasePath: string, sessionId: string): string { return `ws://${uri.authority}/sessions/${sessionId}/channels`; } -import { DapComm } from './DapClient'; - export class KCApi implements PositronSupervisorApi { readonly implementations = { DapComm diff --git a/extensions/positron-supervisor/src/extension.ts b/extensions/positron-supervisor/src/extension.ts index e051e3afd59d..7d60bfee264f 100644 --- a/extensions/positron-supervisor/src/extension.ts +++ b/extensions/positron-supervisor/src/extension.ts @@ -8,7 +8,7 @@ import * as positron from 'positron'; import { PositronSupervisorApi } from './positron-supervisor'; import { KCApi } from './KallichoreAdapterApi'; -import { DapComm } from './DapClient'; +import { DapComm } from './DapComm'; /** Singleton instance of the Kallichore API wrapper */ export let API_INSTANCE: KCApi; From 9621560e120138cc2000360ece332b4a3aef3e44 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 19 Aug 2025 11:18:08 +0200 Subject: [PATCH 24/48] Streamline and document `DapComm` in supervisor API --- .../src/client/positron-supervisor.d.ts | 152 +++++++++++++++--- .../positron-r/src/positron-supervisor.d.ts | 115 +++++++++++-- extensions/positron-r/src/session.ts | 14 +- .../src/positron-supervisor.d.ts | 127 +++++++++++++-- extensions/positron-supervisor/justfile | 5 + extensions/positron-supervisor/src/DapComm.ts | 11 +- .../src/KallichoreAdapterApi.ts | 5 +- .../positron-supervisor/src/extension.ts | 9 +- .../src/positron-supervisor.d.ts | 124 +++++++++++--- 9 files changed, 466 insertions(+), 96 deletions(-) create mode 100644 extensions/positron-supervisor/justfile diff --git a/extensions/positron-python/src/client/positron-supervisor.d.ts b/extensions/positron-python/src/client/positron-supervisor.d.ts index e5e78624e2c2..dbd7fd904c62 100644 --- a/extensions/positron-python/src/client/positron-supervisor.d.ts +++ b/extensions/positron-python/src/client/positron-supervisor.d.ts @@ -69,7 +69,7 @@ export interface JupyterKernelSpec { argv: Array; /** The kernel's display name */ - display_name: string; // eslint-disable-line + display_name: string; // eslint-disable-line /** The language the kernel executes */ language: string; @@ -91,7 +91,7 @@ export interface JupyterKernelSpec { /** Function that starts the kernel given a JupyterSession object. * This is used to start the kernel if it's provided. In this case `argv` * is ignored. - */ + */ startKernel?: (session: JupyterSession, kernel: JupyterKernel) => Promise; } @@ -135,10 +135,7 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * @param target_name Comm type, also used to generate comm identifier. * @param params Optionally, additional parameters included in `comm_open`. */ - createComm( - target_name: string, - params?: Record, - ): Promise; + createComm(target_name: string, params?: Record): Promise; /** * Method for emitting a message to the language server's Jupyter output @@ -182,7 +179,6 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * The Positron Supervisor API as exposed by the Positron Supervisor extension. */ export interface PositronSupervisorApi extends vscode.Disposable { - /** * Create a session for a Jupyter-compatible kernel. * @@ -224,6 +220,16 @@ export interface PositronSupervisorApi extends vscode.Disposable { sessionMetadata: positron.RuntimeSessionMetadata, dynState: positron.LanguageRuntimeDynState, ): Promise; + + /** + * The DAP comm class. + * + * Wraps a raw server comm (see `createServerComm()`) and provides an optional + * `handleMessage()` method for the standard DAP messages. + * + * Must be disposed. Disposing closes the comm if not already done. + */ + readonly DapComm: typeof DapComm; } /** Specific functionality implemented by runtimes */ @@ -241,12 +247,12 @@ export interface JupyterKernelExtra { * Raw comm unmanaged by Positron. * * This type of comm is not mapped to a Positron client. It lives entirely in - * the extension space and allows private communication between an extension and - * its kernel. + * the extension space and allows a direct line of communication between an + * extension and its kernel. * * It's a disposable. Dispose of it once it's closed or you're no longer using * it. If the comm has not already been closed by the kernel, a client-initiated - * `comm_close` message is emitted. + * `comm_close` message is emitted to clean the comm on the backend side. */ export interface RawComm { /** The comm ID. */ @@ -258,34 +264,128 @@ export interface RawComm { /** Send a notification to the backend comm. Returns `false` if comm was closed. */ notify: (method: string, params?: Record) => boolean; - /** Make a request to the backend comm. Resolves when backend responds. The tuple's - * first value indicates whether the comm was closed (and no request was emitted). - * The second value is the result if the request was made. */ + /** + * Make a request to the backend comm. + * + * Resolves when backend responds with a length-2 tuple: + * - A boolean that indicates whether the comm was closed and the request + * could not be emitted. + * - The result if the request was performed. + */ request: (method: string, params?: Record) => Promise<[boolean, any]>; /** Clear resources and sends `comm_close` to backend comm (unless the channel - * was closed by the backend already). */ + * was closed by the backend already). */ dispose: () => void; } /** - * Communication channel. Dispose to close. + * Async-iterable receiver channel for comm messages from the backend. + * The messages are buffered and must be received as long as the channel is open. + * Dispose to close. */ -export interface Channel extends AsyncIterable, vscode.Disposable { } +export interface Channel extends AsyncIterable, vscode.Disposable {} -/** Message from the backend. +/** + * Message from the backend. * * If a request, the `handle` method _must_ be called. + * Throw an error from `handle` to reject the request (e.g. if `method` is unknown). */ export type CommBackendMessage = | { - kind: 'request'; - method: string; - params?: Record; - handle: (handler: () => any) => void; - } + kind: 'request'; + method: string; + params?: Record; + handle: (handler: () => any) => void; + } | { - kind: 'notification'; - method: string; - params?: Record; - }; + kind: 'notification'; + method: string; + params?: Record; + }; + +/** + * A Debug Adapter Protocol (DAP) comm. + * + * This wraps a raw comm that: + * + * - Implements the server protocol (see `createComm()` and + * `JupyterLanguageRuntimeSession::createServerComm()`). + * + * - Optionally handles a standard set of DAP comm messages. + */ +export class DapComm { + /** + * Constructs a new DapComm instance. + * + * @param session The Jupyter language runtime session. + * @param targetName The name of the comm target. + * @param debugType The type of debugger, as required by `vscode.DebugConfiguration.type`. + * @param debugName The name of the debugger, as required by `vscode.DebugConfiguration.name`. + */ + constructor(session: JupyterLanguageRuntimeSession, targetName: string, debugType: string, debugName: string); + + /** The `targetName` passed to the constructor. */ + readonly targetName: string; + + /** The `debugType` passed to the constructor. */ + readonly debugType: string; + + /** The `debugName` passed to the constructor. */ + readonly debugName: string; + + /** + * The raw comm for the DAP. + * Use it to receive messages or make notifications and requests. + * Defined after `createServerComm()` has been called. + */ + readonly comm?: RawComm; + + /** + * The port on which the DAP server is listening. + * Defined after `createServerComm()` has been called. + */ + readonly serverPort?: number; + + /** + * Crate the raw server comm. + * + * Calls `JupyterLanguageRuntimeSession::createServerComm()`. The backend + * comm handling for `targetName` is expected to start a DAP server and + * communicate the port as part of the handshake. + * + * Once resolved: + * - The DAP is ready to accept connections on the backend side. + * - `comm` and `serverPort` are defined. + */ + createComm(): Promise; + + /** + * Handle a message received via `this.comm.receiver`. + * + * This is optional. If called, these message types are handled: + * + * - `start_debug`: A debugging session is started from the frontend side, + * connecting to `this.serverPort`. + * + * - `execute`: A command is visibly executed in the console. Can be used to + * handle DAP requests like "step" via the console, delegating to the + * interpreter's own debugging infrastructure. + * + * - `restart`: The console session is restarted. Can be used to handle a + * restart DAP request on the backend side. + * + * Returns whether the message was handled. Note that if the message was not + * handled, you _must_ check whether the message is a request, and either + * handle or reject it in that case. + */ + handleMessage(msg: any): boolean; + + /** + * Dispose of the underlying comm. + * Must be called if the DAP comm is no longer in use. + * Closes the comm if not done already. + */ + dispose(): void; +} diff --git a/extensions/positron-r/src/positron-supervisor.d.ts b/extensions/positron-r/src/positron-supervisor.d.ts index 91f8248acd18..a63d5aee0996 100644 --- a/extensions/positron-r/src/positron-supervisor.d.ts +++ b/extensions/positron-r/src/positron-supervisor.d.ts @@ -224,6 +224,16 @@ export interface PositronSupervisorApi extends vscode.Disposable { sessionMetadata: positron.RuntimeSessionMetadata, dynState: positron.LanguageRuntimeDynState, ): Promise; + + /** + * The DAP comm class. + * + * Wraps a raw server comm (see `createServerComm()`) and provides an optional + * `handleMessage()` method for the standard DAP messages. + * + * Must be disposed. Disposing closes the comm if not already done. + */ + readonly DapComm: typeof DapComm; } /** Specific functionality implemented by runtimes */ @@ -241,12 +251,12 @@ export interface JupyterKernelExtra { * Raw comm unmanaged by Positron. * * This type of comm is not mapped to a Positron client. It lives entirely in - * the extension space and allows private communication between an extension and - * its kernel. + * the extension space and allows a direct line of communication between an + * extension and its kernel. * * It's a disposable. Dispose of it once it's closed or you're no longer using * it. If the comm has not already been closed by the kernel, a client-initiated - * `comm_close` message is emitted. + * `comm_close` message is emitted to clean the comm on the backend side. */ export interface RawComm { /** The comm ID. */ @@ -258,24 +268,33 @@ export interface RawComm { /** Send a notification to the backend comm. Returns `false` if comm was closed. */ notify: (method: string, params?: Record) => boolean; - /** Make a request to the backend comm. Resolves when backend responds. The tuple's - * first value indicates whether the comm was closed (and no request was emitted). - * The second value is the result if the request was made. */ + /** + * Make a request to the backend comm. + * + * Resolves when backend responds with a length-2 tuple: + * - A boolean that indicates whether the comm was closed and the request + * could not be emitted. + * - The result if the request was performed. + */ request: (method: string, params?: Record) => Promise<[boolean, any]>; /** Clear resources and sends `comm_close` to backend comm (unless the channel - * was closed by the backend already). */ + * was closed by the backend already). */ dispose: () => void; } /** - * Communication channel. Dispose to close. + * Async-iterable receiver channel for comm messages from the backend. + * The messages are buffered and must be received as long as the channel is open. + * Dispose to close. */ export interface Channel extends AsyncIterable, vscode.Disposable { } -/** Message from the backend. +/** + * Message from the backend. * * If a request, the `handle` method _must_ be called. + * Throw an error from `handle` to reject the request (e.g. if `method` is unknown). */ export type CommBackendMessage = | { @@ -291,17 +310,91 @@ export type CommBackendMessage = }; /** - * Interface for DAP communication instances + * A Debug Adapter Protocol (DAP) comm. + * + * This wraps a raw comm that: + * + * - Implements the server protocol (see `createComm()` and + * `JupyterLanguageRuntimeSession::createServerComm()`). + * + * - Optionally handles a standard set of DAP comm messages. */ -export interface DapCommInterface { +export class DapComm { + /** + * Constructs a new DapComm instance. + * + * @param session The Jupyter language runtime session. + * @param targetName The name of the comm target. + * @param debugType The type of debugger, as required by `vscode.DebugConfiguration.type`. + * @param debugName The name of the debugger, as required by `vscode.DebugConfiguration.name`. + */ + constructor( + session: JupyterLanguageRuntimeSession, + targetName: string, + debugType: string, + debugName: string, + ); + + /** The `targetName` passed to the constructor. */ readonly targetName: string; + + /** The `debugType` passed to the constructor. */ readonly debugType: string; + + /** The `debugName` passed to the constructor. */ readonly debugName: string; + /** + * The raw comm for the DAP. + * Use it to receive messages or make notifications and requests. + * Defined after `createServerComm()` has been called. + */ readonly comm?: RawComm; + + /** + * The port on which the DAP server is listening. + * Defined after `createServerComm()` has been called. + */ readonly serverPort?: number; + /** + * Crate the raw server comm. + * + * Calls `JupyterLanguageRuntimeSession::createServerComm()`. The backend + * comm handling for `targetName` is expected to start a DAP server and + * communicate the port as part of the handshake. + * + * Once resolved: + * - The DAP is ready to accept connections on the backend side. + * - `comm` and `serverPort` are defined. + */ createComm(): Promise; + + /** + * Handle a message received via `this.comm.receiver`. + * + * This is optional. If called, these message types are handled: + * + * - `start_debug`: A debugging session is started from the frontend side, + * connecting to `this.serverPort`. + * + * - `execute`: A command is visibly executed in the console. Can be used to + * handle DAP requests like "step" via the console, delegating to the + * interpreter's own debugging infrastructure. + * + * - `restart`: The console session is restarted. Can be used to handle a + * restart DAP request on the backend side. + * + * Returns whether the message was handled. Note that if the message was not + * handled, you _must_ check whether the message is a request, and either + * handle or reject it in that case. + */ handleMessage(msg: any): boolean; + + /** + * Dispose of the underlying comm. + * Must be called if the DAP comm is no longer in use. + * Closes the comm if not done already. + */ dispose(): void; } diff --git a/extensions/positron-r/src/session.ts b/extensions/positron-r/src/session.ts index 43cc7954ae2a..f5c7af821112 100644 --- a/extensions/positron-r/src/session.ts +++ b/extensions/positron-r/src/session.ts @@ -7,7 +7,7 @@ import * as positron from 'positron'; import * as vscode from 'vscode'; import PQueue from 'p-queue'; -import { PositronSupervisorApi, JupyterKernelSpec, JupyterLanguageRuntimeSession, JupyterKernelExtra, RawComm, DapCommInterface } from './positron-supervisor'; +import { PositronSupervisorApi, JupyterKernelSpec, JupyterLanguageRuntimeSession, JupyterKernelExtra, DapComm } from './positron-supervisor'; import { ArkLsp, ArkLspState } from './lsp'; import { delay, whenTimeout, timeout, PromiseHandles } from './util'; import { ArkAttachOnStartup, ArkDelayStartup } from './startup'; @@ -62,7 +62,7 @@ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposa private _kernel?: JupyterLanguageRuntimeSession; /** The DAP communication channel */ - private _dapComm?: DapCommInterface; + private _dapComm?: DapComm; /** The emitter for language runtime messages */ private _messageEmitter = @@ -842,16 +842,10 @@ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposa } const supervisorApi = await supervisorExt.activate(); - if (!supervisorApi.implementations) { - throw new Error('Supervisor API implementations not available'); - } - const dapComm = supervisorApi.implementations.DapComm; - if (!dapComm) { - throw new Error('DapComm implementation not available'); - } - this._dapComm = new dapComm(this._kernel!, 'ark_dap', 'ark', 'Ark Positron R'); + this._dapComm = new supervisorApi.DapComm(this._kernel!, 'ark_dap', 'ark', 'Ark Positron R'); await this._dapComm!.createComm(); + this.startDapMessageLoop(); } catch (err) { LOGGER.error(`Error starting DAP: ${err}`); diff --git a/extensions/positron-reticulate/src/positron-supervisor.d.ts b/extensions/positron-reticulate/src/positron-supervisor.d.ts index 95f61db2d29c..a63d5aee0996 100644 --- a/extensions/positron-reticulate/src/positron-supervisor.d.ts +++ b/extensions/positron-reticulate/src/positron-supervisor.d.ts @@ -224,6 +224,16 @@ export interface PositronSupervisorApi extends vscode.Disposable { sessionMetadata: positron.RuntimeSessionMetadata, dynState: positron.LanguageRuntimeDynState, ): Promise; + + /** + * The DAP comm class. + * + * Wraps a raw server comm (see `createServerComm()`) and provides an optional + * `handleMessage()` method for the standard DAP messages. + * + * Must be disposed. Disposing closes the comm if not already done. + */ + readonly DapComm: typeof DapComm; } /** Specific functionality implemented by runtimes */ @@ -241,12 +251,12 @@ export interface JupyterKernelExtra { * Raw comm unmanaged by Positron. * * This type of comm is not mapped to a Positron client. It lives entirely in - * the extension space and allows private communication between an extension and - * its kernel. + * the extension space and allows a direct line of communication between an + * extension and its kernel. * * It's a disposable. Dispose of it once it's closed or you're no longer using * it. If the comm has not already been closed by the kernel, a client-initiated - * `comm_close` message is emitted. + * `comm_close` message is emitted to clean the comm on the backend side. */ export interface RawComm { /** The comm ID. */ @@ -258,24 +268,33 @@ export interface RawComm { /** Send a notification to the backend comm. Returns `false` if comm was closed. */ notify: (method: string, params?: Record) => boolean; - /** Make a request to the backend comm. Resolves when backend responds. The tuple's - * first value indicates whether the comm was closed (and no request was emitted). - * The second value is the result if the request was made. */ + /** + * Make a request to the backend comm. + * + * Resolves when backend responds with a length-2 tuple: + * - A boolean that indicates whether the comm was closed and the request + * could not be emitted. + * - The result if the request was performed. + */ request: (method: string, params?: Record) => Promise<[boolean, any]>; /** Clear resources and sends `comm_close` to backend comm (unless the channel - * was closed by the backend already). */ + * was closed by the backend already). */ dispose: () => void; } /** - * Communication channel. Dispose to close. + * Async-iterable receiver channel for comm messages from the backend. + * The messages are buffered and must be received as long as the channel is open. + * Dispose to close. */ export interface Channel extends AsyncIterable, vscode.Disposable { } -/** Message from the backend. +/** + * Message from the backend. * * If a request, the `handle` method _must_ be called. + * Throw an error from `handle` to reject the request (e.g. if `method` is unknown). */ export type CommBackendMessage = | { @@ -289,3 +308,93 @@ export type CommBackendMessage = method: string; params?: Record; }; + +/** + * A Debug Adapter Protocol (DAP) comm. + * + * This wraps a raw comm that: + * + * - Implements the server protocol (see `createComm()` and + * `JupyterLanguageRuntimeSession::createServerComm()`). + * + * - Optionally handles a standard set of DAP comm messages. + */ +export class DapComm { + /** + * Constructs a new DapComm instance. + * + * @param session The Jupyter language runtime session. + * @param targetName The name of the comm target. + * @param debugType The type of debugger, as required by `vscode.DebugConfiguration.type`. + * @param debugName The name of the debugger, as required by `vscode.DebugConfiguration.name`. + */ + constructor( + session: JupyterLanguageRuntimeSession, + targetName: string, + debugType: string, + debugName: string, + ); + + /** The `targetName` passed to the constructor. */ + readonly targetName: string; + + /** The `debugType` passed to the constructor. */ + readonly debugType: string; + + /** The `debugName` passed to the constructor. */ + readonly debugName: string; + + /** + * The raw comm for the DAP. + * Use it to receive messages or make notifications and requests. + * Defined after `createServerComm()` has been called. + */ + readonly comm?: RawComm; + + /** + * The port on which the DAP server is listening. + * Defined after `createServerComm()` has been called. + */ + readonly serverPort?: number; + + /** + * Crate the raw server comm. + * + * Calls `JupyterLanguageRuntimeSession::createServerComm()`. The backend + * comm handling for `targetName` is expected to start a DAP server and + * communicate the port as part of the handshake. + * + * Once resolved: + * - The DAP is ready to accept connections on the backend side. + * - `comm` and `serverPort` are defined. + */ + createComm(): Promise; + + /** + * Handle a message received via `this.comm.receiver`. + * + * This is optional. If called, these message types are handled: + * + * - `start_debug`: A debugging session is started from the frontend side, + * connecting to `this.serverPort`. + * + * - `execute`: A command is visibly executed in the console. Can be used to + * handle DAP requests like "step" via the console, delegating to the + * interpreter's own debugging infrastructure. + * + * - `restart`: The console session is restarted. Can be used to handle a + * restart DAP request on the backend side. + * + * Returns whether the message was handled. Note that if the message was not + * handled, you _must_ check whether the message is a request, and either + * handle or reject it in that case. + */ + handleMessage(msg: any): boolean; + + /** + * Dispose of the underlying comm. + * Must be called if the DAP comm is no longer in use. + * Closes the comm if not done already. + */ + dispose(): void; +} diff --git a/extensions/positron-supervisor/justfile b/extensions/positron-supervisor/justfile new file mode 100644 index 000000000000..d714c92c43d2 --- /dev/null +++ b/extensions/positron-supervisor/justfile @@ -0,0 +1,5 @@ +d-ts: + cp src/positron-supervisor.d.ts ../positron-r/src/positron-supervisor.d.ts + cp src/positron-supervisor.d.ts ../positron-reticulate/src/positron-supervisor.d.ts + cp src/positron-supervisor.d.ts ../positron-python/src/client/positron-supervisor.d.ts + npx prettier --config ../positron-python/.prettierrc.js --write ../positron-python/src/client/positron-supervisor.d.ts diff --git a/extensions/positron-supervisor/src/DapComm.ts b/extensions/positron-supervisor/src/DapComm.ts index f3d5613ccdc8..05faad58827b 100644 --- a/extensions/positron-supervisor/src/DapComm.ts +++ b/extensions/positron-supervisor/src/DapComm.ts @@ -8,8 +8,8 @@ import * as positron from 'positron'; import { JupyterLanguageRuntimeSession, RawComm } from './positron-supervisor'; /** - * A Debug Adapter Protocol (DAP) client instance; handles messages from the - * kernel side of the DAP and forwards them to the debug adapter. + * A Debug Adapter Protocol (DAP) comm. + * See `positron-supervisor.d.ts` for documentation. */ export class DapComm { public get comm(): RawComm | undefined { @@ -22,10 +22,10 @@ export class DapComm { private _comm?: RawComm; private _port?: number; - /** Message counter; used for creating unique message IDs */ + // Message counter used for creating unique message IDs private messageCounter = 0; - /** Random stem for messages */ + // Random stem for messages private msgStem: string; constructor( @@ -113,9 +113,6 @@ export class DapComm { } } - /** - * Dispose of the underlying comm, if present. - */ dispose(): void { this._comm?.dispose(); } diff --git a/extensions/positron-supervisor/src/KallichoreAdapterApi.ts b/extensions/positron-supervisor/src/KallichoreAdapterApi.ts index 644c4691feaa..1a620efee10c 100644 --- a/extensions/positron-supervisor/src/KallichoreAdapterApi.ts +++ b/extensions/positron-supervisor/src/KallichoreAdapterApi.ts @@ -122,9 +122,8 @@ function constructWebSocketUri(apiBasePath: string, sessionId: string): string { } export class KCApi implements PositronSupervisorApi { - readonly implementations = { - DapComm - }; + /** The DAP comm class */ + readonly DapComm = DapComm; /** The instance of the API; the API is code-generated from the Kallichore * OpenAPI spec */ diff --git a/extensions/positron-supervisor/src/extension.ts b/extensions/positron-supervisor/src/extension.ts index 7d60bfee264f..e9ea00e8e11f 100644 --- a/extensions/positron-supervisor/src/extension.ts +++ b/extensions/positron-supervisor/src/extension.ts @@ -8,7 +8,6 @@ import * as positron from 'positron'; import { PositronSupervisorApi } from './positron-supervisor'; import { KCApi } from './KallichoreAdapterApi'; -import { DapComm } from './DapComm'; /** Singleton instance of the Kallichore API wrapper */ export let API_INSTANCE: KCApi; @@ -25,13 +24,7 @@ export function activate(context: vscode.ExtensionContext): PositronSupervisorAp log.show(); })); - // Create extended API that includes implementations - const extendedApi = Object.create(API_INSTANCE); - extendedApi.implementations = { - DapComm - }; - - return extendedApi; + return API_INSTANCE; } export function deactivate() { diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index 87e06b82c681..a63d5aee0996 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -226,11 +226,14 @@ export interface PositronSupervisorApi extends vscode.Disposable { ): Promise; /** - * Implementation classes exposed for use by other extensions + * The DAP comm class. + * + * Wraps a raw server comm (see `createServerComm()`) and provides an optional + * `handleMessage()` method for the standard DAP messages. + * + * Must be disposed. Disposing closes the comm if not already done. */ - readonly implementations: { - DapComm: DapCommConstructor; - }; + readonly DapComm: typeof DapComm; } /** Specific functionality implemented by runtimes */ @@ -248,12 +251,12 @@ export interface JupyterKernelExtra { * Raw comm unmanaged by Positron. * * This type of comm is not mapped to a Positron client. It lives entirely in - * the extension space and allows private communication between an extension and - * its kernel. + * the extension space and allows a direct line of communication between an + * extension and its kernel. * * It's a disposable. Dispose of it once it's closed or you're no longer using * it. If the comm has not already been closed by the kernel, a client-initiated - * `comm_close` message is emitted. + * `comm_close` message is emitted to clean the comm on the backend side. */ export interface RawComm { /** The comm ID. */ @@ -265,9 +268,14 @@ export interface RawComm { /** Send a notification to the backend comm. Returns `false` if comm was closed. */ notify: (method: string, params?: Record) => boolean; - /** Make a request to the backend comm. Resolves when backend responds. The tuple's - * first value indicates whether the comm was closed (and no request was emitted). - * The second value is the result if the request was made. */ + /** + * Make a request to the backend comm. + * + * Resolves when backend responds with a length-2 tuple: + * - A boolean that indicates whether the comm was closed and the request + * could not be emitted. + * - The result if the request was performed. + */ request: (method: string, params?: Record) => Promise<[boolean, any]>; /** Clear resources and sends `comm_close` to backend comm (unless the channel @@ -276,13 +284,17 @@ export interface RawComm { } /** - * Communication channel. Dispose to close. + * Async-iterable receiver channel for comm messages from the backend. + * The messages are buffered and must be received as long as the channel is open. + * Dispose to close. */ export interface Channel extends AsyncIterable, vscode.Disposable { } -/** Message from the backend. +/** + * Message from the backend. * * If a request, the `handle` method _must_ be called. + * Throw an error from `handle` to reject the request (e.g. if `method` is unknown). */ export type CommBackendMessage = | { @@ -297,24 +309,92 @@ export type CommBackendMessage = params?: Record; }; -export interface DapCommInterface { +/** + * A Debug Adapter Protocol (DAP) comm. + * + * This wraps a raw comm that: + * + * - Implements the server protocol (see `createComm()` and + * `JupyterLanguageRuntimeSession::createServerComm()`). + * + * - Optionally handles a standard set of DAP comm messages. + */ +export class DapComm { + /** + * Constructs a new DapComm instance. + * + * @param session The Jupyter language runtime session. + * @param targetName The name of the comm target. + * @param debugType The type of debugger, as required by `vscode.DebugConfiguration.type`. + * @param debugName The name of the debugger, as required by `vscode.DebugConfiguration.name`. + */ + constructor( + session: JupyterLanguageRuntimeSession, + targetName: string, + debugType: string, + debugName: string, + ); + + /** The `targetName` passed to the constructor. */ readonly targetName: string; + + /** The `debugType` passed to the constructor. */ readonly debugType: string; + + /** The `debugName` passed to the constructor. */ readonly debugName: string; + /** + * The raw comm for the DAP. + * Use it to receive messages or make notifications and requests. + * Defined after `createServerComm()` has been called. + */ readonly comm?: RawComm; + + /** + * The port on which the DAP server is listening. + * Defined after `createServerComm()` has been called. + */ readonly serverPort?: number; + /** + * Crate the raw server comm. + * + * Calls `JupyterLanguageRuntimeSession::createServerComm()`. The backend + * comm handling for `targetName` is expected to start a DAP server and + * communicate the port as part of the handshake. + * + * Once resolved: + * - The DAP is ready to accept connections on the backend side. + * - `comm` and `serverPort` are defined. + */ createComm(): Promise; + + /** + * Handle a message received via `this.comm.receiver`. + * + * This is optional. If called, these message types are handled: + * + * - `start_debug`: A debugging session is started from the frontend side, + * connecting to `this.serverPort`. + * + * - `execute`: A command is visibly executed in the console. Can be used to + * handle DAP requests like "step" via the console, delegating to the + * interpreter's own debugging infrastructure. + * + * - `restart`: The console session is restarted. Can be used to handle a + * restart DAP request on the backend side. + * + * Returns whether the message was handled. Note that if the message was not + * handled, you _must_ check whether the message is a request, and either + * handle or reject it in that case. + */ handleMessage(msg: any): boolean; - dispose(): void; -} -export interface DapCommConstructor { - new( - session: JupyterLanguageRuntimeSession, - targetName: string, - debugType: string, - debugName: string, - ): DapCommInterface; + /** + * Dispose of the underlying comm. + * Must be called if the DAP comm is no longer in use. + * Closes the comm if not done already. + */ + dispose(): void; } From 32d9a52af5089cde3482ebcbe9a167e8597e4433 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 19 Aug 2025 11:52:18 +0200 Subject: [PATCH 25/48] Make `handleMessage()` async --- .../positron-python/src/client/positron-supervisor.d.ts | 2 +- extensions/positron-r/src/positron-supervisor.d.ts | 2 +- extensions/positron-r/src/session.ts | 2 +- extensions/positron-reticulate/src/positron-supervisor.d.ts | 2 +- extensions/positron-supervisor/src/DapComm.ts | 6 +++--- extensions/positron-supervisor/src/positron-supervisor.d.ts | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/extensions/positron-python/src/client/positron-supervisor.d.ts b/extensions/positron-python/src/client/positron-supervisor.d.ts index dbd7fd904c62..a9c27911eac4 100644 --- a/extensions/positron-python/src/client/positron-supervisor.d.ts +++ b/extensions/positron-python/src/client/positron-supervisor.d.ts @@ -380,7 +380,7 @@ export class DapComm { * handled, you _must_ check whether the message is a request, and either * handle or reject it in that case. */ - handleMessage(msg: any): boolean; + handleMessage(msg: any): Promise; /** * Dispose of the underlying comm. diff --git a/extensions/positron-r/src/positron-supervisor.d.ts b/extensions/positron-r/src/positron-supervisor.d.ts index a63d5aee0996..57bea7f71fbf 100644 --- a/extensions/positron-r/src/positron-supervisor.d.ts +++ b/extensions/positron-r/src/positron-supervisor.d.ts @@ -389,7 +389,7 @@ export class DapComm { * handled, you _must_ check whether the message is a request, and either * handle or reject it in that case. */ - handleMessage(msg: any): boolean; + handleMessage(msg: any): Promise; /** * Dispose of the underlying comm. diff --git a/extensions/positron-r/src/session.ts b/extensions/positron-r/src/session.ts index f5c7af821112..03c0962a0dc1 100644 --- a/extensions/positron-r/src/session.ts +++ b/extensions/positron-r/src/session.ts @@ -866,7 +866,7 @@ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposa for await (const message of this._dapComm.comm.receiver) { LOGGER.trace('Received DAP message:', JSON.stringify(message)); - if (!this._dapComm.handleMessage(message)) { + if (!await this._dapComm.handleMessage(message)) { LOGGER.info(`Unknown DAP message: ${message.method}`); if (message.kind === 'request') { diff --git a/extensions/positron-reticulate/src/positron-supervisor.d.ts b/extensions/positron-reticulate/src/positron-supervisor.d.ts index a63d5aee0996..57bea7f71fbf 100644 --- a/extensions/positron-reticulate/src/positron-supervisor.d.ts +++ b/extensions/positron-reticulate/src/positron-supervisor.d.ts @@ -389,7 +389,7 @@ export class DapComm { * handled, you _must_ check whether the message is a request, and either * handle or reject it in that case. */ - handleMessage(msg: any): boolean; + handleMessage(msg: any): Promise; /** * Dispose of the underlying comm. diff --git a/extensions/positron-supervisor/src/DapComm.ts b/extensions/positron-supervisor/src/DapComm.ts index 05faad58827b..a1f6acfddca8 100644 --- a/extensions/positron-supervisor/src/DapComm.ts +++ b/extensions/positron-supervisor/src/DapComm.ts @@ -51,7 +51,7 @@ export class DapComm { this._port = serverPort; } - handleMessage(msg: any): boolean { + async handleMessage(msg: any): Promise { if (msg.kind === 'request') { return false; } @@ -74,7 +74,7 @@ export class DapComm { // https://github.com/posit-dev/positron/blob/71686862/src/vs/workbench/contrib/debug/browser/debugService.ts#L361 // because `hasDebugged` is undefined. try { - vscode.debug.startDebugging(undefined, config); + await vscode.debug.startDebugging(undefined, config); } catch (err) { this.session.emitJupyterLog( `Can't start debug session for DAP server ${this.comm!.id}: ${err}`, @@ -103,7 +103,7 @@ export class DapComm { // We use the restart button as a shortcut for restarting the runtime case 'restart': { - this.session.restart(); + await this.session.restart(); return true; } diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index a63d5aee0996..57bea7f71fbf 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -389,7 +389,7 @@ export class DapComm { * handled, you _must_ check whether the message is a request, and either * handle or reject it in that case. */ - handleMessage(msg: any): boolean; + handleMessage(msg: any): Promise; /** * Dispose of the underlying comm. From 1419051e6dc4bd10330602482952f7b3ae67264f Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 19 Aug 2025 12:15:05 +0200 Subject: [PATCH 26/48] Review comm documentation --- .../src/client/positron-supervisor.d.ts | 48 +++++++++++++++---- .../positron-r/src/positron-supervisor.d.ts | 48 +++++++++++++++---- .../src/positron-supervisor.d.ts | 48 +++++++++++++++---- .../src/KallichoreSession.ts | 9 +--- .../src/positron-supervisor.d.ts | 48 +++++++++++++++---- 5 files changed, 154 insertions(+), 47 deletions(-) diff --git a/extensions/positron-python/src/client/positron-supervisor.d.ts b/extensions/positron-python/src/client/positron-supervisor.d.ts index a9c27911eac4..d2fbeab1f349 100644 --- a/extensions/positron-python/src/client/positron-supervisor.d.ts +++ b/extensions/positron-python/src/client/positron-supervisor.d.ts @@ -117,26 +117,54 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS */ createPositronLspClientId(): string; - /** - * Creates a server communication channel and returns both the comm and the port. - * - * @param targetName The name of the comm target - * @param host The IP address or host name for the server - * @returns A promise that resolves to a tuple of [RawComm, port number] - */ - createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; - /** * Start a raw comm for communication between frontend and backend. * * Unlike Positron clients, this kind of comm is private to the calling - * extension and its kernel. + * extension and its kernel. They open a direct line of communication that + * lives entirely on the extension host. + * + * The messages sent over these comms are expected to conform to JSON-RPC: + * - The message type is encoded in the `method` field. + * - Parameters are optional and encoded as object (named list of + * parameters) in the `params` field. + * - If a response is expected, add an `id` field to indicate that this is a + * request. This ID field is redundant with the one used in the Jupyter + * layer but allows applications to make a distinction between notifications + * and requests. + * + * Responses to requests follow this format: + * - `result` or `error` field. + * - `id` field corresponding to the request's `id`. * * @param target_name Comm type, also used to generate comm identifier. * @param params Optionally, additional parameters included in `comm_open`. */ createComm(target_name: string, params?: Record): Promise; + /** + * Create a raw server comm. + * + * Server comms are a special type of raw comms (see `createComm()`) that + * wrap a TCP server (e.g. an LSP or DAP server). The backend is expected to + * handle `comm_open` messages for `targetName` comms in the following way: + * + * - The `comm_open` messages includes an `ip_address` field. The server + * must be started on this addess. The server, and not the client, picks + * the port to prevent race conditions where a port becomes used between + * the time it was picked by the frontend and handled by the backend. + * + * - Once the server is started at `ip_address` on a port, the backend sends + * back a notification message of type (method) `server_started` that + * includes a field `port`. + * + * @param targetName The name of the comm target + * @param host The IP address or host name for the server + * @returns A promise that resolves to a tuple of [RawComm, port number] + * once the server has been started on the backend side. + */ + createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; + /** * Method for emitting a message to the language server's Jupyter output * channel. diff --git a/extensions/positron-r/src/positron-supervisor.d.ts b/extensions/positron-r/src/positron-supervisor.d.ts index 57bea7f71fbf..4da360fc180b 100644 --- a/extensions/positron-r/src/positron-supervisor.d.ts +++ b/extensions/positron-r/src/positron-supervisor.d.ts @@ -117,20 +117,25 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS */ createPositronLspClientId(): string; - /** - * Creates a server communication channel and returns both the comm and the port. - * - * @param targetName The name of the comm target - * @param host The IP address or host name for the server - * @returns A promise that resolves to a tuple of [RawComm, port number] - */ - createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; - /** * Start a raw comm for communication between frontend and backend. * * Unlike Positron clients, this kind of comm is private to the calling - * extension and its kernel. + * extension and its kernel. They open a direct line of communication that + * lives entirely on the extension host. + * + * The messages sent over these comms are expected to conform to JSON-RPC: + * - The message type is encoded in the `method` field. + * - Parameters are optional and encoded as object (named list of + * parameters) in the `params` field. + * - If a response is expected, add an `id` field to indicate that this is a + * request. This ID field is redundant with the one used in the Jupyter + * layer but allows applications to make a distinction between notifications + * and requests. + * + * Responses to requests follow this format: + * - `result` or `error` field. + * - `id` field corresponding to the request's `id`. * * @param target_name Comm type, also used to generate comm identifier. * @param params Optionally, additional parameters included in `comm_open`. @@ -140,6 +145,29 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS params?: Record, ): Promise; + /** + * Create a raw server comm. + * + * Server comms are a special type of raw comms (see `createComm()`) that + * wrap a TCP server (e.g. an LSP or DAP server). The backend is expected to + * handle `comm_open` messages for `targetName` comms in the following way: + * + * - The `comm_open` messages includes an `ip_address` field. The server + * must be started on this addess. The server, and not the client, picks + * the port to prevent race conditions where a port becomes used between + * the time it was picked by the frontend and handled by the backend. + * + * - Once the server is started at `ip_address` on a port, the backend sends + * back a notification message of type (method) `server_started` that + * includes a field `port`. + * + * @param targetName The name of the comm target + * @param host The IP address or host name for the server + * @returns A promise that resolves to a tuple of [RawComm, port number] + * once the server has been started on the backend side. + */ + createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; + /** * Method for emitting a message to the language server's Jupyter output * channel. diff --git a/extensions/positron-reticulate/src/positron-supervisor.d.ts b/extensions/positron-reticulate/src/positron-supervisor.d.ts index 57bea7f71fbf..4da360fc180b 100644 --- a/extensions/positron-reticulate/src/positron-supervisor.d.ts +++ b/extensions/positron-reticulate/src/positron-supervisor.d.ts @@ -117,20 +117,25 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS */ createPositronLspClientId(): string; - /** - * Creates a server communication channel and returns both the comm and the port. - * - * @param targetName The name of the comm target - * @param host The IP address or host name for the server - * @returns A promise that resolves to a tuple of [RawComm, port number] - */ - createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; - /** * Start a raw comm for communication between frontend and backend. * * Unlike Positron clients, this kind of comm is private to the calling - * extension and its kernel. + * extension and its kernel. They open a direct line of communication that + * lives entirely on the extension host. + * + * The messages sent over these comms are expected to conform to JSON-RPC: + * - The message type is encoded in the `method` field. + * - Parameters are optional and encoded as object (named list of + * parameters) in the `params` field. + * - If a response is expected, add an `id` field to indicate that this is a + * request. This ID field is redundant with the one used in the Jupyter + * layer but allows applications to make a distinction between notifications + * and requests. + * + * Responses to requests follow this format: + * - `result` or `error` field. + * - `id` field corresponding to the request's `id`. * * @param target_name Comm type, also used to generate comm identifier. * @param params Optionally, additional parameters included in `comm_open`. @@ -140,6 +145,29 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS params?: Record, ): Promise; + /** + * Create a raw server comm. + * + * Server comms are a special type of raw comms (see `createComm()`) that + * wrap a TCP server (e.g. an LSP or DAP server). The backend is expected to + * handle `comm_open` messages for `targetName` comms in the following way: + * + * - The `comm_open` messages includes an `ip_address` field. The server + * must be started on this addess. The server, and not the client, picks + * the port to prevent race conditions where a port becomes used between + * the time it was picked by the frontend and handled by the backend. + * + * - Once the server is started at `ip_address` on a port, the backend sends + * back a notification message of type (method) `server_started` that + * includes a field `port`. + * + * @param targetName The name of the comm target + * @param host The IP address or host name for the server + * @returns A promise that resolves to a tuple of [RawComm, port number] + * once the server has been started on the backend side. + */ + createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; + /** * Method for emitting a message to the language server's Jupyter output * channel. diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 0ae5a5690305..a00968d6e273 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -462,13 +462,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { return `positron-lsp-${this.runtimeMetadata.languageId}-${createUniqueId()}`; } - /** - * Creates a server communication channel and returns both the comm and the port. - * - * @param target_name The name of the comm target - * @param host The IP address or host name for the server - * @returns A promise that resolves to a tuple of [RawComm, port number] - */ + /** Create a raw server comm. See `positron-supervisor.d.ts` for documentation. */ async createServerComm(target_name: string, host: string): Promise<[RawComm, number]> { this.log(`Starting server comm '${target_name}' for ${host}`); const comm = await this.createComm(target_name, { ip_address: host }); @@ -725,6 +719,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { } } + /** Create raw comm. See `positron-supervisor.d.ts` for documentation. */ async createComm( target_name: string, params: Record = {}, diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index 57bea7f71fbf..4da360fc180b 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -117,20 +117,25 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS */ createPositronLspClientId(): string; - /** - * Creates a server communication channel and returns both the comm and the port. - * - * @param targetName The name of the comm target - * @param host The IP address or host name for the server - * @returns A promise that resolves to a tuple of [RawComm, port number] - */ - createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; - /** * Start a raw comm for communication between frontend and backend. * * Unlike Positron clients, this kind of comm is private to the calling - * extension and its kernel. + * extension and its kernel. They open a direct line of communication that + * lives entirely on the extension host. + * + * The messages sent over these comms are expected to conform to JSON-RPC: + * - The message type is encoded in the `method` field. + * - Parameters are optional and encoded as object (named list of + * parameters) in the `params` field. + * - If a response is expected, add an `id` field to indicate that this is a + * request. This ID field is redundant with the one used in the Jupyter + * layer but allows applications to make a distinction between notifications + * and requests. + * + * Responses to requests follow this format: + * - `result` or `error` field. + * - `id` field corresponding to the request's `id`. * * @param target_name Comm type, also used to generate comm identifier. * @param params Optionally, additional parameters included in `comm_open`. @@ -140,6 +145,29 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS params?: Record, ): Promise; + /** + * Create a raw server comm. + * + * Server comms are a special type of raw comms (see `createComm()`) that + * wrap a TCP server (e.g. an LSP or DAP server). The backend is expected to + * handle `comm_open` messages for `targetName` comms in the following way: + * + * - The `comm_open` messages includes an `ip_address` field. The server + * must be started on this addess. The server, and not the client, picks + * the port to prevent race conditions where a port becomes used between + * the time it was picked by the frontend and handled by the backend. + * + * - Once the server is started at `ip_address` on a port, the backend sends + * back a notification message of type (method) `server_started` that + * includes a field `port`. + * + * @param targetName The name of the comm target + * @param host The IP address or host name for the server + * @returns A promise that resolves to a tuple of [RawComm, port number] + * once the server has been started on the backend side. + */ + createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; + /** * Method for emitting a message to the language server's Jupyter output * channel. From 7390a8b51945adf29bf88a42e3da75627ad4c51a Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 19 Aug 2025 13:57:57 +0200 Subject: [PATCH 27/48] Rename `Channel` to `ReceiverChannel` --- .../src/client/positron-supervisor.d.ts | 23 ++----------------- .../positron-r/src/positron-supervisor.d.ts | 23 ++----------------- .../src/positron-supervisor.d.ts | 23 ++----------------- .../src/KallichoreSession.ts | 2 +- .../src/positron-supervisor.d.ts | 23 ++----------------- 5 files changed, 9 insertions(+), 85 deletions(-) diff --git a/extensions/positron-python/src/client/positron-supervisor.d.ts b/extensions/positron-python/src/client/positron-supervisor.d.ts index d2fbeab1f349..600dc90b73b1 100644 --- a/extensions/positron-python/src/client/positron-supervisor.d.ts +++ b/extensions/positron-python/src/client/positron-supervisor.d.ts @@ -34,25 +34,6 @@ export interface JupyterKernel { log(msg: string): void; } -/** - * Message sent from the frontend requesting a server to start - */ -export interface ServerStartMessage { - /** The IP address or host name on which the client is listening for server requests. The server is - * in charge of picking the exact port to communicate over, since the server is the - * one that binds to the port (to prevent race conditions). - */ - host: string; -} - -/** - * Message sent to the frontend to acknowledge that the corresponding server has started - */ -export interface ServerStartedMessage { - /** The port that the frontend should connect to on the `ip_address` it sent over */ - port: number; -} - /** * This set of type definitions defines the interfaces used by the Positron * Supervisor extension. @@ -287,7 +268,7 @@ export interface RawComm { id: string; /** Async-iterable for messages sent from backend. */ - receiver: Channel; + receiver: ReceiverChannel; /** Send a notification to the backend comm. Returns `false` if comm was closed. */ notify: (method: string, params?: Record) => boolean; @@ -312,7 +293,7 @@ export interface RawComm { * The messages are buffered and must be received as long as the channel is open. * Dispose to close. */ -export interface Channel extends AsyncIterable, vscode.Disposable {} +export interface ReceiverChannel extends AsyncIterable, vscode.Disposable {} /** * Message from the backend. diff --git a/extensions/positron-r/src/positron-supervisor.d.ts b/extensions/positron-r/src/positron-supervisor.d.ts index 4da360fc180b..7b400f20cdd3 100644 --- a/extensions/positron-r/src/positron-supervisor.d.ts +++ b/extensions/positron-r/src/positron-supervisor.d.ts @@ -34,25 +34,6 @@ export interface JupyterKernel { log(msg: string): void; } -/** - * Message sent from the frontend requesting a server to start - */ -export interface ServerStartMessage { - /** The IP address or host name on which the client is listening for server requests. The server is - * in charge of picking the exact port to communicate over, since the server is the - * one that binds to the port (to prevent race conditions). - */ - host: string; -} - -/** - * Message sent to the frontend to acknowledge that the corresponding server has started - */ -export interface ServerStartedMessage { - /** The port that the frontend should connect to on the `ip_address` it sent over */ - port: number; -} - /** * This set of type definitions defines the interfaces used by the Positron * Supervisor extension. @@ -291,7 +272,7 @@ export interface RawComm { id: string; /** Async-iterable for messages sent from backend. */ - receiver: Channel; + receiver: ReceiverChannel; /** Send a notification to the backend comm. Returns `false` if comm was closed. */ notify: (method: string, params?: Record) => boolean; @@ -316,7 +297,7 @@ export interface RawComm { * The messages are buffered and must be received as long as the channel is open. * Dispose to close. */ -export interface Channel extends AsyncIterable, vscode.Disposable { } +export interface ReceiverChannel extends AsyncIterable, vscode.Disposable { } /** * Message from the backend. diff --git a/extensions/positron-reticulate/src/positron-supervisor.d.ts b/extensions/positron-reticulate/src/positron-supervisor.d.ts index 4da360fc180b..7b400f20cdd3 100644 --- a/extensions/positron-reticulate/src/positron-supervisor.d.ts +++ b/extensions/positron-reticulate/src/positron-supervisor.d.ts @@ -34,25 +34,6 @@ export interface JupyterKernel { log(msg: string): void; } -/** - * Message sent from the frontend requesting a server to start - */ -export interface ServerStartMessage { - /** The IP address or host name on which the client is listening for server requests. The server is - * in charge of picking the exact port to communicate over, since the server is the - * one that binds to the port (to prevent race conditions). - */ - host: string; -} - -/** - * Message sent to the frontend to acknowledge that the corresponding server has started - */ -export interface ServerStartedMessage { - /** The port that the frontend should connect to on the `ip_address` it sent over */ - port: number; -} - /** * This set of type definitions defines the interfaces used by the Positron * Supervisor extension. @@ -291,7 +272,7 @@ export interface RawComm { id: string; /** Async-iterable for messages sent from backend. */ - receiver: Channel; + receiver: ReceiverChannel; /** Send a notification to the backend comm. Returns `false` if comm was closed. */ notify: (method: string, params?: Record) => boolean; @@ -316,7 +297,7 @@ export interface RawComm { * The messages are buffered and must be received as long as the channel is open. * Dispose to close. */ -export interface Channel extends AsyncIterable, vscode.Disposable { } +export interface ReceiverChannel extends AsyncIterable, vscode.Disposable { } /** * Message from the backend. diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index a00968d6e273..70471293a8a7 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -8,7 +8,7 @@ import * as positron from 'positron'; import * as os from 'os'; import * as path from 'path'; import * as fs from 'fs'; -import { CommBackendMessage, JupyterKernelExtra, JupyterKernelSpec, JupyterLanguageRuntimeSession, JupyterSession, RawComm, ServerStartMessage, ServerStartedMessage } from './positron-supervisor'; +import { CommBackendMessage, JupyterKernelExtra, JupyterKernelSpec, JupyterLanguageRuntimeSession, JupyterSession, RawComm } from './positron-supervisor'; import { ActiveSession, ConnectionInfo, DefaultApi, HttpError, InterruptMode, NewSession, RestartSession, Status, VarAction, VarActionType } from './kcclient/api'; import { JupyterMessage } from './jupyter/JupyterMessage'; import { JupyterRequest } from './jupyter/JupyterRequest'; diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index 4da360fc180b..7b400f20cdd3 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -34,25 +34,6 @@ export interface JupyterKernel { log(msg: string): void; } -/** - * Message sent from the frontend requesting a server to start - */ -export interface ServerStartMessage { - /** The IP address or host name on which the client is listening for server requests. The server is - * in charge of picking the exact port to communicate over, since the server is the - * one that binds to the port (to prevent race conditions). - */ - host: string; -} - -/** - * Message sent to the frontend to acknowledge that the corresponding server has started - */ -export interface ServerStartedMessage { - /** The port that the frontend should connect to on the `ip_address` it sent over */ - port: number; -} - /** * This set of type definitions defines the interfaces used by the Positron * Supervisor extension. @@ -291,7 +272,7 @@ export interface RawComm { id: string; /** Async-iterable for messages sent from backend. */ - receiver: Channel; + receiver: ReceiverChannel; /** Send a notification to the backend comm. Returns `false` if comm was closed. */ notify: (method: string, params?: Record) => boolean; @@ -316,7 +297,7 @@ export interface RawComm { * The messages are buffered and must be received as long as the channel is open. * Dispose to close. */ -export interface Channel extends AsyncIterable, vscode.Disposable { } +export interface ReceiverChannel extends AsyncIterable, vscode.Disposable { } /** * Message from the backend. From 74148bc4f9d92d0210fead64d7e37b5dfba6df8b Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 19 Aug 2025 14:08:56 +0200 Subject: [PATCH 28/48] Add `supervisorApi()` accessor --- extensions/positron-r/src/extension.ts | 14 +++++++++++++ extensions/positron-r/src/runtime-manager.ts | 12 +++-------- extensions/positron-r/src/session.ts | 22 ++++---------------- 3 files changed, 21 insertions(+), 27 deletions(-) diff --git a/extensions/positron-r/src/extension.ts b/extensions/positron-r/src/extension.ts index eea91c437802..a07a9138a8a5 100644 --- a/extensions/positron-r/src/extension.ts +++ b/extensions/positron-r/src/extension.ts @@ -14,6 +14,7 @@ import { RRuntimeManager } from './runtime-manager'; import { registerUriHandler } from './uri-handler'; import { registerRLanguageModelTools } from './llm-tools.js'; import { registerFileAssociations } from './file-associations.js'; +import { PositronSupervisorApi } from './positron-supervisor'; export const LOGGER = vscode.window.createOutputChannel('R Language Pack', { log: true }); @@ -53,3 +54,16 @@ export function activate(context: vscode.ExtensionContext) { } }); } + +export async function supervisorApi(): Promise { + const ext = vscode.extensions.getExtension('positron.positron-supervisor'); + if (!ext) { + throw new Error('Positron Supervisor extension not found'); + } + + if (!ext.isActive) { + await ext.activate(); + } + + return ext?.exports as PositronSupervisorApi; +} diff --git a/extensions/positron-r/src/runtime-manager.ts b/extensions/positron-r/src/runtime-manager.ts index a73f1c86fb37..44bf1d0e48a0 100644 --- a/extensions/positron-r/src/runtime-manager.ts +++ b/extensions/positron-r/src/runtime-manager.ts @@ -10,7 +10,7 @@ import { currentRBinary, makeMetadata, rRuntimeDiscoverer } from './provider'; import { RInstallation, RMetadataExtra, ReasonDiscovered, friendlyReason } from './r-installation'; import { RSession, createJupyterKernelExtra } from './session'; import { createJupyterKernelSpec } from './kernel-spec'; -import { LOGGER } from './extension'; +import { LOGGER, supervisorApi } from './extension'; import { POSITRON_R_INTERPRETERS_DEFAULT_SETTING_KEY } from './constants'; import { getDefaultInterpreterPath } from './interpreter-settings.js'; import { dirname } from 'path'; @@ -160,14 +160,8 @@ export class RRuntimeManager implements positron.LanguageRuntimeManager { * @returns True if the session is valid, false otherwise */ async validateSession(sessionId: string): Promise { - const ext = vscode.extensions.getExtension('positron.positron-supervisor'); - if (!ext) { - throw new Error('Positron Supervisor extension not found'); - } - if (!ext.isActive) { - await ext.activate(); - } - return ext.exports.validateSession(sessionId); + const api = await supervisorApi(); + return await api.validateSession(sessionId); } restoreSession( diff --git a/extensions/positron-r/src/session.ts b/extensions/positron-r/src/session.ts index 03c0962a0dc1..a6f844ae088d 100644 --- a/extensions/positron-r/src/session.ts +++ b/extensions/positron-r/src/session.ts @@ -15,7 +15,7 @@ import { RHtmlWidget, getResourceRoots } from './htmlwidgets'; import { randomUUID } from 'crypto'; import { handleRCode } from './hyperlink'; import { RSessionManager } from './session-manager'; -import { LOGGER } from './extension.js'; +import { LOGGER, supervisorApi } from './extension.js'; interface RPackageInstallation { packageName: string; @@ -628,15 +628,7 @@ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposa } private async createKernel(): Promise { - // Get the Positron Supervisor extension and activate it if necessary - const ext = vscode.extensions.getExtension('positron.positron-supervisor'); - if (!ext) { - throw new Error('Positron Supervisor extension not found'); - } - if (!ext.isActive) { - await ext.activate(); - } - this.adapterApi = ext?.exports as PositronSupervisorApi; + this.adapterApi = await supervisorApi(); // Create the Jupyter session const kernel = this.kernelSpec ? @@ -835,15 +827,9 @@ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposa private async startDap(): Promise { if (this._kernel) { try { - // Get the supervisor extension API - const supervisorExt = vscode.extensions.getExtension('positron.positron-supervisor'); - if (!supervisorExt) { - throw new Error('Positron Supervisor extension not found'); - } - - const supervisorApi = await supervisorExt.activate(); + const api = await supervisorApi(); - this._dapComm = new supervisorApi.DapComm(this._kernel!, 'ark_dap', 'ark', 'Ark Positron R'); + this._dapComm = new api.DapComm(this._kernel!, 'ark_dap', 'ark', 'Ark Positron R'); await this._dapComm!.createComm(); this.startDapMessageLoop(); From 530b2f18482cd88eedaf877b32e2e71b2e89d423 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 19 Aug 2025 14:16:27 +0200 Subject: [PATCH 29/48] Move check inside --- extensions/positron-r/src/session.ts | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/extensions/positron-r/src/session.ts b/extensions/positron-r/src/session.ts index a6f844ae088d..f006769e87f8 100644 --- a/extensions/positron-r/src/session.ts +++ b/extensions/positron-r/src/session.ts @@ -825,17 +825,19 @@ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposa * sessions by coming online. */ private async startDap(): Promise { - if (this._kernel) { - try { - const api = await supervisorApi(); + try { + if (!this._kernel) { + throw new Error('Kernel not started'); + } - this._dapComm = new api.DapComm(this._kernel!, 'ark_dap', 'ark', 'Ark Positron R'); - await this._dapComm!.createComm(); + const api = await supervisorApi(); - this.startDapMessageLoop(); - } catch (err) { - LOGGER.error(`Error starting DAP: ${err}`); - } + this._dapComm = new api.DapComm(this._kernel, 'ark_dap', 'ark', 'Ark Positron R'); + await this._dapComm!.createComm(); + + this.startDapMessageLoop(); + } catch (err) { + LOGGER.error(`Error starting DAP: ${err}`); } } From d55fc83eddf7d2f27e8fdc331a64cbd68b2ddec7 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Tue, 19 Aug 2025 16:01:56 +0200 Subject: [PATCH 30/48] Await request result --- extensions/positron-supervisor/src/RawComm.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/positron-supervisor/src/RawComm.ts b/extensions/positron-supervisor/src/RawComm.ts index b8769ae0c116..09e2d4bcb117 100644 --- a/extensions/positron-supervisor/src/RawComm.ts +++ b/extensions/positron-supervisor/src/RawComm.ts @@ -60,7 +60,7 @@ export class RawCommImpl implements vscode.Disposable { }; const request = new CommMsgRequest(id, commMsg); - return [true, this.session.sendRequest(request)]; + return [true, await this.session.sendRequest(request)]; } close() { From ca2dc91ec56351c7557e83b850d2f1f48d51a5a4 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Wed, 20 Aug 2025 11:45:15 +0200 Subject: [PATCH 31/48] Don't close if already closed --- extensions/positron-supervisor/src/RawComm.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/extensions/positron-supervisor/src/RawComm.ts b/extensions/positron-supervisor/src/RawComm.ts index 09e2d4bcb117..33984c010930 100644 --- a/extensions/positron-supervisor/src/RawComm.ts +++ b/extensions/positron-supervisor/src/RawComm.ts @@ -68,6 +68,10 @@ export class RawCommImpl implements vscode.Disposable { } closeAndNotify() { + if (this.closed) { + return; + } + this.close(); const commClose = new CommCloseCommand(this.id); this.session.sendCommand(commClose); From 8743714bda7bddf4f382e7c05e1fcbdec75c3cad Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Wed, 20 Aug 2025 09:27:35 +0200 Subject: [PATCH 32/48] Add tests for raw comms And fix a few issues --- .../src/client/positron-supervisor.d.ts | 6 +- extensions/positron-r/src/ark-comm.ts | 35 ++++++++ .../positron-r/src/positron-supervisor.d.ts | 6 +- extensions/positron-r/src/session.ts | 25 ++++++ .../positron-r/src/test/ark-comm.test.ts | 79 +++++++++++++++++++ extensions/positron-r/src/test/kit-session.ts | 3 +- .../src/positron-supervisor.d.ts | 6 +- extensions/positron-supervisor/src/RawComm.ts | 13 ++- .../src/positron-supervisor.d.ts | 6 +- 9 files changed, 167 insertions(+), 12 deletions(-) create mode 100644 extensions/positron-r/src/ark-comm.ts create mode 100644 extensions/positron-r/src/test/ark-comm.test.ts diff --git a/extensions/positron-python/src/client/positron-supervisor.d.ts b/extensions/positron-python/src/client/positron-supervisor.d.ts index 600dc90b73b1..8a440d5c7ddc 100644 --- a/extensions/positron-python/src/client/positron-supervisor.d.ts +++ b/extensions/positron-python/src/client/positron-supervisor.d.ts @@ -285,7 +285,7 @@ export interface RawComm { /** Clear resources and sends `comm_close` to backend comm (unless the channel * was closed by the backend already). */ - dispose: () => void; + dispose: () => Promise; } /** @@ -293,7 +293,9 @@ export interface RawComm { * The messages are buffered and must be received as long as the channel is open. * Dispose to close. */ -export interface ReceiverChannel extends AsyncIterable, vscode.Disposable {} +export interface ReceiverChannel extends AsyncIterable, vscode.Disposable { + next(): Promise>; +} /** * Message from the backend. diff --git a/extensions/positron-r/src/ark-comm.ts b/extensions/positron-r/src/ark-comm.ts new file mode 100644 index 000000000000..e23046edbd80 --- /dev/null +++ b/extensions/positron-r/src/ark-comm.ts @@ -0,0 +1,35 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2025 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as vscode from 'vscode'; +import { JupyterLanguageRuntimeSession, RawComm } from './positron-supervisor'; +import { LOGGER } from './extension'; + +/** + * Communication channel with the bakend. + * Currently only used for testing. + */ +export class ArkComm implements vscode.Disposable { + readonly targetName: string = 'ark'; + + public get comm(): RawComm | undefined { + return this._comm; + } + + private _comm?: RawComm; + + constructor( + private session: JupyterLanguageRuntimeSession, + ) { } + + async createComm(): Promise { + this._comm = await this.session.createComm(this.targetName); + LOGGER.info(`Created Ark comm with ID: ${this._comm.id}`); + } + + async dispose(): Promise { + await this._comm?.dispose(); + } +} diff --git a/extensions/positron-r/src/positron-supervisor.d.ts b/extensions/positron-r/src/positron-supervisor.d.ts index 7b400f20cdd3..3529c74a6375 100644 --- a/extensions/positron-r/src/positron-supervisor.d.ts +++ b/extensions/positron-r/src/positron-supervisor.d.ts @@ -289,7 +289,7 @@ export interface RawComm { /** Clear resources and sends `comm_close` to backend comm (unless the channel * was closed by the backend already). */ - dispose: () => void; + dispose: () => Promise; } /** @@ -297,7 +297,9 @@ export interface RawComm { * The messages are buffered and must be received as long as the channel is open. * Dispose to close. */ -export interface ReceiverChannel extends AsyncIterable, vscode.Disposable { } +export interface ReceiverChannel extends AsyncIterable, vscode.Disposable { + next(): Promise>; +} /** * Message from the backend. diff --git a/extensions/positron-r/src/session.ts b/extensions/positron-r/src/session.ts index f006769e87f8..482f2553b690 100644 --- a/extensions/positron-r/src/session.ts +++ b/extensions/positron-r/src/session.ts @@ -16,6 +16,7 @@ import { randomUUID } from 'crypto'; import { handleRCode } from './hyperlink'; import { RSessionManager } from './session-manager'; import { LOGGER, supervisorApi } from './extension.js'; +import { ArkComm } from './ark-comm'; interface RPackageInstallation { packageName: string; @@ -45,6 +46,13 @@ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposa /** The Language Server Protocol client wrapper */ private _lsp: ArkLsp; + /** The Ark Comm for direct communication with the kernel */ + private _arkComm?: ArkComm; + + get arkComm(): ArkComm | undefined { + return this._arkComm; + } + /** Queue for LSP events */ private _lspQueue: PQueue; @@ -394,6 +402,9 @@ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposa this._consoleWidthDisposable = undefined; await this._lsp.dispose(); + if (this._arkComm) { + await this._arkComm.dispose(); + } if (this._kernel) { await this._kernel.dispose(); } @@ -841,6 +852,20 @@ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposa } } + // Only called from tests for now + async startArkComm(): Promise { + try { + if (!this._kernel) { + throw new Error('Kernel not started'); + } + + this._arkComm = new ArkComm(this._kernel); + await this._arkComm!.createComm(); + } catch (err) { + LOGGER.error(`Error starting DAP: ${err}`); + } + } + /** * Handle DAP messages in an infinite loop */ diff --git a/extensions/positron-r/src/test/ark-comm.test.ts b/extensions/positron-r/src/test/ark-comm.test.ts new file mode 100644 index 000000000000..552681ca80be --- /dev/null +++ b/extensions/positron-r/src/test/ark-comm.test.ts @@ -0,0 +1,79 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2024-2025 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import * as positron from 'positron'; +import * as vscode from 'vscode'; +import { ArkComm } from '../ark-comm'; +import * as testKit from './kit'; +import { RSession } from '../session'; +import { RawComm, CommBackendMessage } from '../positron-supervisor'; +import { whenTimeout } from '../util'; + +suite('ArkComm', () => { + let session: RSession; + let sesDisposable: vscode.Disposable; + let comm: RawComm; + + suiteSetup(async () => { + const [ses, disposable] = await testKit.startR(); + session = ses; + sesDisposable = disposable; + + await session.startArkComm(); + assert.notStrictEqual(session.arkComm, undefined); + assert.notStrictEqual(session.arkComm!.comm, undefined); + + comm = session.arkComm!.comm!; + }); + + suiteTeardown(async () => { + if (sesDisposable) { + await sesDisposable.dispose(); + } + }); + + test('Can send notification', async () => { + assert(comm.notify("test_notification", { i: 10 })); + + // Backend should echo back + const notifReply = await assertNextMessage(comm); + assert.deepStrictEqual( + notifReply, + { + kind: 'notification', + method: 'test_notification', + params: { + i: -10 + } + } + ) + }); + + test('Can send request', async () => { + const requestReply = await assertRequest(comm, 'test_request', { i: 11 }); + assert.deepStrictEqual(requestReply, { i: -11 }) + }); +}); + +async function assertNextMessage(comm: RawComm): Promise { + const result = await Promise.race([ + comm.receiver.next(), + whenTimeout(5000, () => assert.fail(`Timeout while expecting comm message on ${comm.id}`)), + ]) as any; + + assert.strictEqual(result.done, false) + return result.value; +} + +async function assertRequest(comm: RawComm, method: string, params?: Record): Promise { + const [delivered, reply] = await Promise.race([ + comm.request(method, params), + whenTimeout(5000, () => assert.fail(`Timeout while expecting comm reply on ${comm.id}`)), + ]); + + assert.strictEqual(delivered, true); + return reply; +} diff --git a/extensions/positron-r/src/test/kit-session.ts b/extensions/positron-r/src/test/kit-session.ts index 0250d3297968..0baa66779d1f 100644 --- a/extensions/positron-r/src/test/kit-session.ts +++ b/extensions/positron-r/src/test/kit-session.ts @@ -39,8 +39,7 @@ export async function startR(): Promise<[RSession, vscode.Disposable, ArkLsp]> { const lspReady = session.waitLsp(); const lspTimeout = (async () => { - await delay(2000); - undefined + await delay(5000); })(); const lsp = await Promise.race([lspReady, lspTimeout]); diff --git a/extensions/positron-reticulate/src/positron-supervisor.d.ts b/extensions/positron-reticulate/src/positron-supervisor.d.ts index 7b400f20cdd3..3529c74a6375 100644 --- a/extensions/positron-reticulate/src/positron-supervisor.d.ts +++ b/extensions/positron-reticulate/src/positron-supervisor.d.ts @@ -289,7 +289,7 @@ export interface RawComm { /** Clear resources and sends `comm_close` to backend comm (unless the channel * was closed by the backend already). */ - dispose: () => void; + dispose: () => Promise; } /** @@ -297,7 +297,9 @@ export interface RawComm { * The messages are buffered and must be received as long as the channel is open. * Dispose to close. */ -export interface ReceiverChannel extends AsyncIterable, vscode.Disposable { } +export interface ReceiverChannel extends AsyncIterable, vscode.Disposable { + next(): Promise>; +} /** * Message from the backend. diff --git a/extensions/positron-supervisor/src/RawComm.ts b/extensions/positron-supervisor/src/RawComm.ts index 33984c010930..535236851c3d 100644 --- a/extensions/positron-supervisor/src/RawComm.ts +++ b/extensions/positron-supervisor/src/RawComm.ts @@ -60,7 +60,16 @@ export class RawCommImpl implements vscode.Disposable { }; const request = new CommMsgRequest(id, commMsg); - return [true, await this.session.sendRequest(request)]; + const reply = await this.session.sendRequest(request); + + if (reply.data.error !== undefined) { + throw new Error('TODO'); + } + if (reply.data.result === undefined) { + throw new Error(`Internal error in ${this.id}: undefined result for request ${msg}`); + } + + return [true, reply.data.result]; } close() { @@ -77,7 +86,7 @@ export class RawCommImpl implements vscode.Disposable { this.session.sendCommand(commClose); } - dispose() { + async dispose(): Promise { this.close(); for (const disposable of this.disposables) { disposable.dispose(); diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index 7b400f20cdd3..3529c74a6375 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -289,7 +289,7 @@ export interface RawComm { /** Clear resources and sends `comm_close` to backend comm (unless the channel * was closed by the backend already). */ - dispose: () => void; + dispose: () => Promise; } /** @@ -297,7 +297,9 @@ export interface RawComm { * The messages are buffered and must be received as long as the channel is open. * Dispose to close. */ -export interface ReceiverChannel extends AsyncIterable, vscode.Disposable { } +export interface ReceiverChannel extends AsyncIterable, vscode.Disposable { + next(): Promise>; +} /** * Message from the backend. From 023c2aa310fd4f6e7f88e287ad041f443724ecf6 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Wed, 20 Aug 2025 13:54:09 +0200 Subject: [PATCH 33/48] Note that requests from backend to frontend are currently not possible --- extensions/positron-python/src/client/positron-supervisor.d.ts | 3 +++ extensions/positron-r/src/positron-supervisor.d.ts | 3 +++ extensions/positron-reticulate/src/positron-supervisor.d.ts | 3 +++ extensions/positron-supervisor/src/positron-supervisor.d.ts | 3 +++ 4 files changed, 12 insertions(+) diff --git a/extensions/positron-python/src/client/positron-supervisor.d.ts b/extensions/positron-python/src/client/positron-supervisor.d.ts index 8a440d5c7ddc..2e0f8afe3787 100644 --- a/extensions/positron-python/src/client/positron-supervisor.d.ts +++ b/extensions/positron-python/src/client/positron-supervisor.d.ts @@ -302,6 +302,9 @@ export interface ReceiverChannel extends AsyncIterable, vscode.Disposable * * If a request, the `handle` method _must_ be called. * Throw an error from `handle` to reject the request (e.g. if `method` is unknown). + * + * Note: Requests are currently not possible, see + * */ export type CommBackendMessage = | { diff --git a/extensions/positron-r/src/positron-supervisor.d.ts b/extensions/positron-r/src/positron-supervisor.d.ts index 3529c74a6375..5b8bd5d7b6a1 100644 --- a/extensions/positron-r/src/positron-supervisor.d.ts +++ b/extensions/positron-r/src/positron-supervisor.d.ts @@ -306,6 +306,9 @@ export interface ReceiverChannel extends AsyncIterable, vscode.Disposable * * If a request, the `handle` method _must_ be called. * Throw an error from `handle` to reject the request (e.g. if `method` is unknown). + * + * Note: Requests are currently not possible, see + * */ export type CommBackendMessage = | { diff --git a/extensions/positron-reticulate/src/positron-supervisor.d.ts b/extensions/positron-reticulate/src/positron-supervisor.d.ts index 3529c74a6375..5b8bd5d7b6a1 100644 --- a/extensions/positron-reticulate/src/positron-supervisor.d.ts +++ b/extensions/positron-reticulate/src/positron-supervisor.d.ts @@ -306,6 +306,9 @@ export interface ReceiverChannel extends AsyncIterable, vscode.Disposable * * If a request, the `handle` method _must_ be called. * Throw an error from `handle` to reject the request (e.g. if `method` is unknown). + * + * Note: Requests are currently not possible, see + * */ export type CommBackendMessage = | { diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index 3529c74a6375..5b8bd5d7b6a1 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -306,6 +306,9 @@ export interface ReceiverChannel extends AsyncIterable, vscode.Disposable * * If a request, the `handle` method _must_ be called. * Throw an error from `handle` to reject the request (e.g. if `method` is unknown). + * + * Note: Requests are currently not possible, see + * */ export type CommBackendMessage = | { From ff71b2e3850d527c9de05219a2f6b687d6272004 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Wed, 20 Aug 2025 14:09:45 +0200 Subject: [PATCH 34/48] Throw errors from `notify()` and `request()` --- .../src/client/positron-supervisor.d.ts | 40 +++++++++-- .../positron-r/src/positron-supervisor.d.ts | 40 +++++++++-- .../positron-r/src/test/ark-comm.test.ts | 31 ++++++-- .../src/positron-supervisor.d.ts | 40 +++++++++-- extensions/positron-supervisor/src/RawComm.ts | 71 ++++++++++++++----- .../src/positron-supervisor.d.ts | 40 +++++++++-- 6 files changed, 210 insertions(+), 52 deletions(-) diff --git a/extensions/positron-python/src/client/positron-supervisor.d.ts b/extensions/positron-python/src/client/positron-supervisor.d.ts index 2e0f8afe3787..361f95d07732 100644 --- a/extensions/positron-python/src/client/positron-supervisor.d.ts +++ b/extensions/positron-python/src/client/positron-supervisor.d.ts @@ -270,18 +270,21 @@ export interface RawComm { /** Async-iterable for messages sent from backend. */ receiver: ReceiverChannel; - /** Send a notification to the backend comm. Returns `false` if comm was closed. */ - notify: (method: string, params?: Record) => boolean; + /** + * Send a notification to the backend comm. + * Throws `CommClosedError` if comm was closed. + */ + notify: (method: string, params?: Record) => void; /** * Make a request to the backend comm. * - * Resolves when backend responds with a length-2 tuple: - * - A boolean that indicates whether the comm was closed and the request - * could not be emitted. - * - The result if the request was performed. + * Resolves when backend responds with the result. + * Throws: + * - `CommClosedError` if comm was closed + * - `CommRpcError` for RPC errors. */ - request: (method: string, params?: Record) => Promise<[boolean, any]>; + request: (method: string, params?: Record) => Promise; /** Clear resources and sends `comm_close` to backend comm (unless the channel * was closed by the backend already). */ @@ -297,6 +300,29 @@ export interface ReceiverChannel extends AsyncIterable, vscode.Disposable next(): Promise>; } +/** + * Base class for communication errors. + */ +export class CommError extends Error { + constructor(message: string, method?: string); + readonly method?: string; +} + +/** + * Error thrown when attempting to communicate through a closed channel. + */ +export class CommClosedError extends CommError { + constructor(commId: string, method?: string); +} + +/** + * Error thrown for RPC-specific errors with error codes. + */ +export class CommRpcError extends CommError { + constructor(message: string, code?: number, method?: string); + readonly code: number; +} + /** * Message from the backend. * diff --git a/extensions/positron-r/src/positron-supervisor.d.ts b/extensions/positron-r/src/positron-supervisor.d.ts index 5b8bd5d7b6a1..983a8a622210 100644 --- a/extensions/positron-r/src/positron-supervisor.d.ts +++ b/extensions/positron-r/src/positron-supervisor.d.ts @@ -274,18 +274,21 @@ export interface RawComm { /** Async-iterable for messages sent from backend. */ receiver: ReceiverChannel; - /** Send a notification to the backend comm. Returns `false` if comm was closed. */ - notify: (method: string, params?: Record) => boolean; + /** + * Send a notification to the backend comm. + * Throws `CommClosedError` if comm was closed. + */ + notify: (method: string, params?: Record) => void; /** * Make a request to the backend comm. * - * Resolves when backend responds with a length-2 tuple: - * - A boolean that indicates whether the comm was closed and the request - * could not be emitted. - * - The result if the request was performed. + * Resolves when backend responds with the result. + * Throws: + * - `CommClosedError` if comm was closed + * - `CommRpcError` for RPC errors. */ - request: (method: string, params?: Record) => Promise<[boolean, any]>; + request: (method: string, params?: Record) => Promise; /** Clear resources and sends `comm_close` to backend comm (unless the channel * was closed by the backend already). */ @@ -301,6 +304,29 @@ export interface ReceiverChannel extends AsyncIterable, vscode.Disposable next(): Promise>; } +/** + * Base class for communication errors. + */ +export declare class CommError extends Error { + name: 'CommError' | 'CommClosedError' | 'CommRpcError'; + readonly method?: string; +} + +/** + * Error thrown when attempting to communicate through a closed channel. + */ +export declare class CommClosedError extends CommError { + name: 'CommClosedError'; +} + +/** + * Error thrown for RPC-specific errors with error codes. + */ +export declare class CommRpcError extends CommError { + name: 'CommRpcError'; + readonly code: number; +} + /** * Message from the backend. * diff --git a/extensions/positron-r/src/test/ark-comm.test.ts b/extensions/positron-r/src/test/ark-comm.test.ts index 552681ca80be..0500ee136421 100644 --- a/extensions/positron-r/src/test/ark-comm.test.ts +++ b/extensions/positron-r/src/test/ark-comm.test.ts @@ -4,9 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as assert from 'assert'; -import * as positron from 'positron'; import * as vscode from 'vscode'; -import { ArkComm } from '../ark-comm'; import * as testKit from './kit'; import { RSession } from '../session'; import { RawComm, CommBackendMessage } from '../positron-supervisor'; @@ -36,7 +34,7 @@ suite('ArkComm', () => { }); test('Can send notification', async () => { - assert(comm.notify("test_notification", { i: 10 })); + comm.notify("test_notification", { i: 10 }); // Backend should echo back const notifReply = await assertNextMessage(comm); @@ -56,6 +54,28 @@ suite('ArkComm', () => { const requestReply = await assertRequest(comm, 'test_request', { i: 11 }); assert.deepStrictEqual(requestReply, { i: -11 }) }); + + test('Invalid method sends error', async () => { + await assert.rejects( + async () => { + await assertRequest(comm, 'invalid_request', {}); + }, + (error: any) => { + return error.name === 'CommRpcError'; + } + ); + }); + + test('Request can error', async () => { + await assert.rejects( + async () => { + await assertRequest(comm, 'test_request_error', {}); + }, + (error: any) => { + return error.name === 'CommRpcError' && /this-is-an-error/.test(error.message); + } + ); + }); }); async function assertNextMessage(comm: RawComm): Promise { @@ -69,11 +89,8 @@ async function assertNextMessage(comm: RawComm): Promise { } async function assertRequest(comm: RawComm, method: string, params?: Record): Promise { - const [delivered, reply] = await Promise.race([ + return await Promise.race([ comm.request(method, params), whenTimeout(5000, () => assert.fail(`Timeout while expecting comm reply on ${comm.id}`)), ]); - - assert.strictEqual(delivered, true); - return reply; } diff --git a/extensions/positron-reticulate/src/positron-supervisor.d.ts b/extensions/positron-reticulate/src/positron-supervisor.d.ts index 5b8bd5d7b6a1..2a52f54cccca 100644 --- a/extensions/positron-reticulate/src/positron-supervisor.d.ts +++ b/extensions/positron-reticulate/src/positron-supervisor.d.ts @@ -274,18 +274,21 @@ export interface RawComm { /** Async-iterable for messages sent from backend. */ receiver: ReceiverChannel; - /** Send a notification to the backend comm. Returns `false` if comm was closed. */ - notify: (method: string, params?: Record) => boolean; + /** + * Send a notification to the backend comm. + * Throws `CommClosedError` if comm was closed. + */ + notify: (method: string, params?: Record) => void; /** * Make a request to the backend comm. * - * Resolves when backend responds with a length-2 tuple: - * - A boolean that indicates whether the comm was closed and the request - * could not be emitted. - * - The result if the request was performed. + * Resolves when backend responds with the result. + * Throws: + * - `CommClosedError` if comm was closed + * - `CommRpcError` for RPC errors. */ - request: (method: string, params?: Record) => Promise<[boolean, any]>; + request: (method: string, params?: Record) => Promise; /** Clear resources and sends `comm_close` to backend comm (unless the channel * was closed by the backend already). */ @@ -301,6 +304,29 @@ export interface ReceiverChannel extends AsyncIterable, vscode.Disposable next(): Promise>; } +/** + * Base class for communication errors. + */ +export class CommError extends Error { + constructor(message: string, method?: string); + readonly method?: string; +} + +/** + * Error thrown when attempting to communicate through a closed channel. + */ +export class CommClosedError extends CommError { + constructor(commId: string, method?: string); +} + +/** + * Error thrown for RPC-specific errors with error codes. + */ +export class CommRpcError extends CommError { + constructor(message: string, code?: number, method?: string); + readonly code: number; +} + /** * Message from the backend. * diff --git a/extensions/positron-supervisor/src/RawComm.ts b/extensions/positron-supervisor/src/RawComm.ts index 535236851c3d..41b75372a41f 100644 --- a/extensions/positron-supervisor/src/RawComm.ts +++ b/extensions/positron-supervisor/src/RawComm.ts @@ -12,6 +12,34 @@ import { Receiver } from './Channel'; import { CommBackendMessage } from './positron-supervisor'; import { CommCloseCommand } from './jupyter/CommCloseCommand'; +export class CommError extends Error { + constructor( + message: string, + public readonly method?: string + ) { + super(message); + this.name = 'CommError'; + } +} + +export class CommClosedError extends CommError { + constructor(commId: string, method?: string) { + super(`Communication channel ${commId} is closed`, method); + this.name = 'CommClosedError'; + } +} + +export class CommRpcError extends CommError { + constructor( + message: string, + public readonly code: number = -32000, + method?: string + ) { + super(message, method); + this.name = 'CommRpcError'; + } +} + export class RawCommImpl implements vscode.Disposable { private readonly disposables: vscode.Disposable[] = []; private closed = false; @@ -22,9 +50,9 @@ export class RawCommImpl implements vscode.Disposable { public readonly receiver: Receiver, ) { } - notify(method: string, params?: Record): boolean { + notify(method: string, params?: Record): void { if (this.closed) { - return false; + throw new CommClosedError(this.id, method); } const msg: CommRpcMessage = { @@ -36,13 +64,11 @@ export class RawCommImpl implements vscode.Disposable { // We don't expect a response here, so `id` can be created and forgotten const id = createUniqueId(); this.session.sendClientMessage(this.id, id, msg); - - return true; } - async request(method: string, params?: Record): Promise<[boolean, any]> { + async request(method: string, params?: Record): Promise { if (this.closed) { - return [false, undefined]; + throw new CommClosedError(this.id, method); } const id = createUniqueId(); @@ -63,13 +89,19 @@ export class RawCommImpl implements vscode.Disposable { const reply = await this.session.sendRequest(request); if (reply.data.error !== undefined) { - throw new Error('TODO'); + const payload = reply.data as CommRpcMessageError; + throw new CommRpcError( + payload.error.message, + payload.error.code, + method + ); } + if (reply.data.result === undefined) { throw new Error(`Internal error in ${this.id}: undefined result for request ${msg}`); } - return [true, reply.data.result]; + return reply.data.result; } close() { @@ -130,7 +162,7 @@ export class CommBackendRequest { } reply(result: any) { - const msg: CommRpcResponse = { + const msg: CommRpcMessageResult = { jsonrpc: '2.0', id: this.id, method: this.method, @@ -140,12 +172,14 @@ export class CommBackendRequest { } reject(error: Error, code = -32000) { - const msg: CommRpcError = { + const msg: CommRpcMessageError = { jsonrpc: '2.0', id: this.id, - method: this.method, - message: `${error}`, - code, + error: { + method: this.method, + message: `${error}`, + code, + } }; this.send(msg); } @@ -169,17 +203,20 @@ export interface CommRpcMessage { [key: string]: unknown; } -interface CommRpcResponse { +interface CommRpcMessageResult { jsonrpc: '2.0'; result: any; id: string; [key: string]: unknown; } -interface CommRpcError { +interface CommRpcMessageError { jsonrpc: '2.0'; - message: string; - code: number; + error: { + message: string; + code: number; + [key: string]: unknown; + }; id: string; [key: string]: unknown; } diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index 5b8bd5d7b6a1..2a52f54cccca 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -274,18 +274,21 @@ export interface RawComm { /** Async-iterable for messages sent from backend. */ receiver: ReceiverChannel; - /** Send a notification to the backend comm. Returns `false` if comm was closed. */ - notify: (method: string, params?: Record) => boolean; + /** + * Send a notification to the backend comm. + * Throws `CommClosedError` if comm was closed. + */ + notify: (method: string, params?: Record) => void; /** * Make a request to the backend comm. * - * Resolves when backend responds with a length-2 tuple: - * - A boolean that indicates whether the comm was closed and the request - * could not be emitted. - * - The result if the request was performed. + * Resolves when backend responds with the result. + * Throws: + * - `CommClosedError` if comm was closed + * - `CommRpcError` for RPC errors. */ - request: (method: string, params?: Record) => Promise<[boolean, any]>; + request: (method: string, params?: Record) => Promise; /** Clear resources and sends `comm_close` to backend comm (unless the channel * was closed by the backend already). */ @@ -301,6 +304,29 @@ export interface ReceiverChannel extends AsyncIterable, vscode.Disposable next(): Promise>; } +/** + * Base class for communication errors. + */ +export class CommError extends Error { + constructor(message: string, method?: string); + readonly method?: string; +} + +/** + * Error thrown when attempting to communicate through a closed channel. + */ +export class CommClosedError extends CommError { + constructor(commId: string, method?: string); +} + +/** + * Error thrown for RPC-specific errors with error codes. + */ +export class CommRpcError extends CommError { + constructor(message: string, code?: number, method?: string); + readonly code: number; +} + /** * Message from the backend. * From 5cb0d4ec6f3ce16f5e4852a64ed4912f1769ba0a Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Wed, 20 Aug 2025 16:47:58 +0200 Subject: [PATCH 35/48] Rename `Comm` to `Client` --- .../positron-supervisor/src/{Comm.ts => Client.ts} | 11 +++++------ .../positron-supervisor/src/KallichoreSession.ts | 8 ++++---- 2 files changed, 9 insertions(+), 10 deletions(-) rename extensions/positron-supervisor/src/{Comm.ts => Client.ts} (63%) diff --git a/extensions/positron-supervisor/src/Comm.ts b/extensions/positron-supervisor/src/Client.ts similarity index 63% rename from extensions/positron-supervisor/src/Comm.ts rename to extensions/positron-supervisor/src/Client.ts index ec61e81a94c6..be427964366e 100644 --- a/extensions/positron-supervisor/src/Comm.ts +++ b/extensions/positron-supervisor/src/Client.ts @@ -4,15 +4,14 @@ *--------------------------------------------------------------------------------------------*/ /** - * Simple representation of a comm (communications channel) between the client - * and the kernel + * Simple representation of a client (and its underlying comm) between Positron and the kernel */ -export class Comm { +export class Client { /** - * Create a new comm representation + * Create a new client representation * - * @param id The unique ID of the comm instance @param target The comm - * @param target The comm's target name (also known as its type); can be any + * @param id The unique ID of the comm/client instance. + * @param target The comm/client's target name (also known as its type); can be any * string. Positron-specific comms are listed in its `RuntimeClientType` * enum. */ diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 70471293a8a7..a3433e66a6c1 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -31,7 +31,7 @@ import { JupyterChannel } from './jupyter/JupyterChannel'; import { InputReplyCommand } from './jupyter/InputReplyCommand'; import { RpcReplyCommand } from './jupyter/RpcReplyCommand'; import { JupyterCommRequest } from './jupyter/JupyterCommRequest'; -import { Comm } from './Comm'; +import { Client } from './Client'; import { CommMsgRequest } from './jupyter/CommMsgRequest'; import { SocketSession } from './ws/SocketSession'; import { KernelOutputMessage } from './ws/KernelMessage'; @@ -147,7 +147,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { private _profileChannel: vscode.OutputChannel | undefined; /** A map of active comms connected to Positron clients */ - private readonly _clients: Map = new Map(); + private readonly _clients: Map = new Map(); /** A map of active comms unmanaged by Positron */ private readonly _rawComms: Map]> = new Map(); @@ -782,7 +782,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { }; const commOpen = new CommOpenCommand(msg, metadata); await this.sendCommand(commOpen); - this._clients.set(id, new Comm(id, type)); + this._clients.set(id, new Client(id, type)); // If we have any pending UI comm requests and we just created the // UI comm, send them now @@ -820,7 +820,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { // If we don't have a comm object for this comm, create one if (!this._clients.has(key)) { // Should this be renamed to Client? - this._clients.set(key, new Comm(key, target)); + this._clients.set(key, new Client(key, target)); } // If we just discovered a UI comm, send any pending UI comm From ff603875263254fe847b0a9493027906ffb6e7ce Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 21 Aug 2025 06:39:30 +0200 Subject: [PATCH 36/48] Add comments from pair review with Davis --- .../src/client/positron-supervisor.d.ts | 11 ++++++++- .../positron-r/src/positron-supervisor.d.ts | 23 +++++++++++++------ extensions/positron-r/src/session.ts | 4 +++- .../src/positron-supervisor.d.ts | 11 ++++++++- extensions/positron-supervisor/src/RawComm.ts | 2 ++ .../src/positron-supervisor.d.ts | 11 ++++++++- 6 files changed, 51 insertions(+), 11 deletions(-) diff --git a/extensions/positron-python/src/client/positron-supervisor.d.ts b/extensions/positron-python/src/client/positron-supervisor.d.ts index 361f95d07732..f47a657de185 100644 --- a/extensions/positron-python/src/client/positron-supervisor.d.ts +++ b/extensions/positron-python/src/client/positron-supervisor.d.ts @@ -267,7 +267,16 @@ export interface RawComm { /** The comm ID. */ id: string; - /** Async-iterable for messages sent from backend. */ + /** + * Async-iterable for messages sent from backend. + * + * - This receiver channel _must_ be awaited and handled to exhaustion. + * - When exhausted, you _must_ dispose of the comm. + * + * Yields `CommBackendMessage` messages which are a tagged union of + * notifications and requests. If a request, the `handle` method _must_ be + * called (see `CommBackendMessage` documentation). + */ receiver: ReceiverChannel; /** diff --git a/extensions/positron-r/src/positron-supervisor.d.ts b/extensions/positron-r/src/positron-supervisor.d.ts index 983a8a622210..2adb8864f05a 100644 --- a/extensions/positron-r/src/positron-supervisor.d.ts +++ b/extensions/positron-r/src/positron-supervisor.d.ts @@ -271,7 +271,16 @@ export interface RawComm { /** The comm ID. */ id: string; - /** Async-iterable for messages sent from backend. */ + /** + * Async-iterable for messages sent from backend. + * + * - This receiver channel _must_ be awaited and handled to exhaustion. + * - When exhausted, you _must_ dispose of the comm. + * + * Yields `CommBackendMessage` messages which are a tagged union of + * notifications and requests. If a request, the `handle` method _must_ be + * called (see `CommBackendMessage` documentation). + */ receiver: ReceiverChannel; /** @@ -307,23 +316,23 @@ export interface ReceiverChannel extends AsyncIterable, vscode.Disposable /** * Base class for communication errors. */ -export declare class CommError extends Error { - name: 'CommError' | 'CommClosedError' | 'CommRpcError'; +export class CommError extends Error { + constructor(message: string, method?: string); readonly method?: string; } /** * Error thrown when attempting to communicate through a closed channel. */ -export declare class CommClosedError extends CommError { - name: 'CommClosedError'; +export class CommClosedError extends CommError { + constructor(commId: string, method?: string); } /** * Error thrown for RPC-specific errors with error codes. */ -export declare class CommRpcError extends CommError { - name: 'CommRpcError'; +export class CommRpcError extends CommError { + constructor(message: string, code?: number, method?: string); readonly code: number; } diff --git a/extensions/positron-r/src/session.ts b/extensions/positron-r/src/session.ts index 482f2553b690..047760e2e9cf 100644 --- a/extensions/positron-r/src/session.ts +++ b/extensions/positron-r/src/session.ts @@ -846,6 +846,7 @@ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposa this._dapComm = new api.DapComm(this._kernel, 'ark_dap', 'ark', 'Ark Positron R'); await this._dapComm!.createComm(); + // Not awaited: we're spawning an infinite async loop this.startDapMessageLoop(); } catch (err) { LOGGER.error(`Error starting DAP: ${err}`); @@ -867,7 +868,8 @@ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposa } /** - * Handle DAP messages in an infinite loop + * Handle DAP messages in an infinite loop. + * Should typically not be awaited. */ private async startDapMessageLoop(): Promise { LOGGER.info('Starting DAP loop'); diff --git a/extensions/positron-reticulate/src/positron-supervisor.d.ts b/extensions/positron-reticulate/src/positron-supervisor.d.ts index 2a52f54cccca..2adb8864f05a 100644 --- a/extensions/positron-reticulate/src/positron-supervisor.d.ts +++ b/extensions/positron-reticulate/src/positron-supervisor.d.ts @@ -271,7 +271,16 @@ export interface RawComm { /** The comm ID. */ id: string; - /** Async-iterable for messages sent from backend. */ + /** + * Async-iterable for messages sent from backend. + * + * - This receiver channel _must_ be awaited and handled to exhaustion. + * - When exhausted, you _must_ dispose of the comm. + * + * Yields `CommBackendMessage` messages which are a tagged union of + * notifications and requests. If a request, the `handle` method _must_ be + * called (see `CommBackendMessage` documentation). + */ receiver: ReceiverChannel; /** diff --git a/extensions/positron-supervisor/src/RawComm.ts b/extensions/positron-supervisor/src/RawComm.ts index 41b75372a41f..82a78b7c6ada 100644 --- a/extensions/positron-supervisor/src/RawComm.ts +++ b/extensions/positron-supervisor/src/RawComm.ts @@ -118,6 +118,8 @@ export class RawCommImpl implements vscode.Disposable { this.session.sendCommand(commClose); } + // Make sure not to call `dispose()` from Kallichore, only the owner of the + // comm should dispose of it. Kallichore calls the `close()` method instead. async dispose(): Promise { this.close(); for (const disposable of this.disposables) { diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index 2a52f54cccca..2adb8864f05a 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -271,7 +271,16 @@ export interface RawComm { /** The comm ID. */ id: string; - /** Async-iterable for messages sent from backend. */ + /** + * Async-iterable for messages sent from backend. + * + * - This receiver channel _must_ be awaited and handled to exhaustion. + * - When exhausted, you _must_ dispose of the comm. + * + * Yields `CommBackendMessage` messages which are a tagged union of + * notifications and requests. If a request, the `handle` method _must_ be + * called (see `CommBackendMessage` documentation). + */ receiver: ReceiverChannel; /** From 61a2dad10404146259321efc3356ae72199b032c Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 21 Aug 2025 06:48:30 +0200 Subject: [PATCH 37/48] Don't dispose of comm in Kallichore --- extensions/positron-supervisor/src/KallichoreSession.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index a3433e66a6c1..6900ed53e163 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -1943,7 +1943,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { this._rawComms.delete(closeMsg.comm_id); const [comm, _] = commHandle; - comm.dispose(); + comm.close(); return; } From 2d47608a17261fd256acce2ecb27b6966c5f7a88 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 21 Aug 2025 06:49:47 +0200 Subject: [PATCH 38/48] Store `RawCommImpl` instead of `RawComm` To avoid down-casting --- extensions/positron-supervisor/src/KallichoreSession.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 6900ed53e163..c23313aa1be8 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -150,7 +150,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { private readonly _clients: Map = new Map(); /** A map of active comms unmanaged by Positron */ - private readonly _rawComms: Map]> = new Map(); + private readonly _rawComms: Map]> = new Map(); /** The kernel's log file, if any. */ private _kernelLogFile: string | undefined; @@ -723,7 +723,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { async createComm( target_name: string, params: Record = {}, - ): Promise { + ): Promise { const id = `extension-comm-${target_name}-${this.runtimeMetadata.languageId}-${createUniqueId()}`; const [tx, rx] = channel(); @@ -749,7 +749,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { const commOpen = new CommOpenCommand(msg); await this.sendCommand(commOpen); - return comm; + return comm as RawComm; } /** @@ -1780,7 +1780,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { // Close all raw comms for (const [comm, tx] of this._rawComms.values()) { // Don't dispose of comm, this resource is owned by caller of `createComm()`. - (comm as RawCommImpl).close(); + comm.close(); tx.dispose(); } this._rawComms.clear(); From b0b29f3de7a62d1b3137171ca7a55841768cb1a5 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 21 Aug 2025 07:03:38 +0200 Subject: [PATCH 39/48] Don't expose whole DAP comm class through supervisor API --- .../src/client/positron-supervisor.d.ts | 41 +++++++-------- .../positron-r/src/positron-supervisor.d.ts | 50 ++++++++----------- extensions/positron-r/src/session.ts | 5 +- .../src/positron-supervisor.d.ts | 50 ++++++++----------- .../src/KallichoreSession.ts | 12 +++++ .../src/positron-supervisor.d.ts | 50 ++++++++----------- 6 files changed, 97 insertions(+), 111 deletions(-) diff --git a/extensions/positron-python/src/client/positron-supervisor.d.ts b/extensions/positron-python/src/client/positron-supervisor.d.ts index f47a657de185..87f118d86f02 100644 --- a/extensions/positron-python/src/client/positron-supervisor.d.ts +++ b/extensions/positron-python/src/client/positron-supervisor.d.ts @@ -146,6 +146,18 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS */ createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; + /** + * Constructs a new DapComm instance. + * Must be disposed. See `DapComm` documentation. + * + * @param session The Jupyter language runtime session. + * @param targetName The name of the comm target. + * @param debugType The type of debugger, as required by `vscode.DebugConfiguration.type`. + * @param debugName The name of the debugger, as required by `vscode.DebugConfiguration.name`. + * @returns A new `DapComm` instance. + */ + createDapComm(targetName: string, debugType: string, debugName: string): Promise; + /** * Method for emitting a message to the language server's Jupyter output * channel. @@ -229,16 +241,6 @@ export interface PositronSupervisorApi extends vscode.Disposable { sessionMetadata: positron.RuntimeSessionMetadata, dynState: positron.LanguageRuntimeDynState, ): Promise; - - /** - * The DAP comm class. - * - * Wraps a raw server comm (see `createServerComm()`) and provides an optional - * `handleMessage()` method for the standard DAP messages. - * - * Must be disposed. Disposing closes the comm if not already done. - */ - readonly DapComm: typeof DapComm; } /** Specific functionality implemented by runtimes */ @@ -357,24 +359,17 @@ export type CommBackendMessage = /** * A Debug Adapter Protocol (DAP) comm. * - * This wraps a raw comm that: + * This wraps a `Comm` that: * * - Implements the server protocol (see `createComm()` and * `JupyterLanguageRuntimeSession::createServerComm()`). * * - Optionally handles a standard set of DAP comm messages. + * + * Must be disposed when no longer in use or if `comm.receiver` is exhausted. + * Disposing the `DapComm` automatically disposes of the nested `Comm`. */ -export class DapComm { - /** - * Constructs a new DapComm instance. - * - * @param session The Jupyter language runtime session. - * @param targetName The name of the comm target. - * @param debugType The type of debugger, as required by `vscode.DebugConfiguration.type`. - * @param debugName The name of the debugger, as required by `vscode.DebugConfiguration.name`. - */ - constructor(session: JupyterLanguageRuntimeSession, targetName: string, debugType: string, debugName: string); - +export interface DapComm { /** The `targetName` passed to the constructor. */ readonly targetName: string; @@ -398,7 +393,7 @@ export class DapComm { readonly serverPort?: number; /** - * Crate the raw server comm. + * Create the raw server comm. * * Calls `JupyterLanguageRuntimeSession::createServerComm()`. The backend * comm handling for `targetName` is expected to start a DAP server and diff --git a/extensions/positron-r/src/positron-supervisor.d.ts b/extensions/positron-r/src/positron-supervisor.d.ts index 2adb8864f05a..87adb3bd8637 100644 --- a/extensions/positron-r/src/positron-supervisor.d.ts +++ b/extensions/positron-r/src/positron-supervisor.d.ts @@ -149,6 +149,22 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS */ createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; + /** + * Constructs a new DapComm instance. + * Must be disposed. See `DapComm` documentation. + * + * @param session The Jupyter language runtime session. + * @param targetName The name of the comm target. + * @param debugType The type of debugger, as required by `vscode.DebugConfiguration.type`. + * @param debugName The name of the debugger, as required by `vscode.DebugConfiguration.name`. + * @returns A new `DapComm` instance. + */ + createDapComm( + targetName: string, + debugType: string, + debugName: string, + ): Promise; + /** * Method for emitting a message to the language server's Jupyter output * channel. @@ -233,16 +249,6 @@ export interface PositronSupervisorApi extends vscode.Disposable { sessionMetadata: positron.RuntimeSessionMetadata, dynState: positron.LanguageRuntimeDynState, ): Promise; - - /** - * The DAP comm class. - * - * Wraps a raw server comm (see `createServerComm()`) and provides an optional - * `handleMessage()` method for the standard DAP messages. - * - * Must be disposed. Disposing closes the comm if not already done. - */ - readonly DapComm: typeof DapComm; } /** Specific functionality implemented by runtimes */ @@ -361,29 +367,17 @@ export type CommBackendMessage = /** * A Debug Adapter Protocol (DAP) comm. * - * This wraps a raw comm that: + * This wraps a `Comm` that: * * - Implements the server protocol (see `createComm()` and * `JupyterLanguageRuntimeSession::createServerComm()`). * * - Optionally handles a standard set of DAP comm messages. + * + * Must be disposed when no longer in use or if `comm.receiver` is exhausted. + * Disposing the `DapComm` automatically disposes of the nested `Comm`. */ -export class DapComm { - /** - * Constructs a new DapComm instance. - * - * @param session The Jupyter language runtime session. - * @param targetName The name of the comm target. - * @param debugType The type of debugger, as required by `vscode.DebugConfiguration.type`. - * @param debugName The name of the debugger, as required by `vscode.DebugConfiguration.name`. - */ - constructor( - session: JupyterLanguageRuntimeSession, - targetName: string, - debugType: string, - debugName: string, - ); - +export interface DapComm { /** The `targetName` passed to the constructor. */ readonly targetName: string; @@ -407,7 +401,7 @@ export class DapComm { readonly serverPort?: number; /** - * Crate the raw server comm. + * Create the raw server comm. * * Calls `JupyterLanguageRuntimeSession::createServerComm()`. The backend * comm handling for `targetName` is expected to start a DAP server and diff --git a/extensions/positron-r/src/session.ts b/extensions/positron-r/src/session.ts index 047760e2e9cf..4852c5e86b8c 100644 --- a/extensions/positron-r/src/session.ts +++ b/extensions/positron-r/src/session.ts @@ -841,10 +841,7 @@ export class RSession implements positron.LanguageRuntimeSession, vscode.Disposa throw new Error('Kernel not started'); } - const api = await supervisorApi(); - - this._dapComm = new api.DapComm(this._kernel, 'ark_dap', 'ark', 'Ark Positron R'); - await this._dapComm!.createComm(); + this._dapComm = await this._kernel.createDapComm('ark_dap', 'ark', 'Ark Positron R'); // Not awaited: we're spawning an infinite async loop this.startDapMessageLoop(); diff --git a/extensions/positron-reticulate/src/positron-supervisor.d.ts b/extensions/positron-reticulate/src/positron-supervisor.d.ts index 2adb8864f05a..87adb3bd8637 100644 --- a/extensions/positron-reticulate/src/positron-supervisor.d.ts +++ b/extensions/positron-reticulate/src/positron-supervisor.d.ts @@ -149,6 +149,22 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS */ createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; + /** + * Constructs a new DapComm instance. + * Must be disposed. See `DapComm` documentation. + * + * @param session The Jupyter language runtime session. + * @param targetName The name of the comm target. + * @param debugType The type of debugger, as required by `vscode.DebugConfiguration.type`. + * @param debugName The name of the debugger, as required by `vscode.DebugConfiguration.name`. + * @returns A new `DapComm` instance. + */ + createDapComm( + targetName: string, + debugType: string, + debugName: string, + ): Promise; + /** * Method for emitting a message to the language server's Jupyter output * channel. @@ -233,16 +249,6 @@ export interface PositronSupervisorApi extends vscode.Disposable { sessionMetadata: positron.RuntimeSessionMetadata, dynState: positron.LanguageRuntimeDynState, ): Promise; - - /** - * The DAP comm class. - * - * Wraps a raw server comm (see `createServerComm()`) and provides an optional - * `handleMessage()` method for the standard DAP messages. - * - * Must be disposed. Disposing closes the comm if not already done. - */ - readonly DapComm: typeof DapComm; } /** Specific functionality implemented by runtimes */ @@ -361,29 +367,17 @@ export type CommBackendMessage = /** * A Debug Adapter Protocol (DAP) comm. * - * This wraps a raw comm that: + * This wraps a `Comm` that: * * - Implements the server protocol (see `createComm()` and * `JupyterLanguageRuntimeSession::createServerComm()`). * * - Optionally handles a standard set of DAP comm messages. + * + * Must be disposed when no longer in use or if `comm.receiver` is exhausted. + * Disposing the `DapComm` automatically disposes of the nested `Comm`. */ -export class DapComm { - /** - * Constructs a new DapComm instance. - * - * @param session The Jupyter language runtime session. - * @param targetName The name of the comm target. - * @param debugType The type of debugger, as required by `vscode.DebugConfiguration.type`. - * @param debugName The name of the debugger, as required by `vscode.DebugConfiguration.name`. - */ - constructor( - session: JupyterLanguageRuntimeSession, - targetName: string, - debugType: string, - debugName: string, - ); - +export interface DapComm { /** The `targetName` passed to the constructor. */ readonly targetName: string; @@ -407,7 +401,7 @@ export class DapComm { readonly serverPort?: number; /** - * Crate the raw server comm. + * Create the raw server comm. * * Calls `JupyterLanguageRuntimeSession::createServerComm()`. The backend * comm handling for `targetName` is expected to start a DAP server and diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index c23313aa1be8..90fcc5a22e4d 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -43,6 +43,7 @@ import { JupyterMessageType } from './jupyter/JupyterMessageType.js'; import { JupyterCommClose } from './jupyter/JupyterCommClose'; import { CommBackendRequest, CommRpcMessage, RawCommImpl } from './RawComm'; import { channel, Sender } from './Channel'; +import { DapComm } from './DapComm'; /** * The reason for a disconnection event. @@ -752,6 +753,17 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { return comm as RawComm; } + /** Create DAP comm. See `positron-supervisor.d.ts` for documentation. */ + async createDapComm( + targetName: string, + debugType: string, + debugName: string, + ): Promise { + const comm = new DapComm(this, targetName, debugType, debugName); + await comm.createComm(); + return comm; + } + /** * Create a new client comm. * diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index 2adb8864f05a..87adb3bd8637 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -149,6 +149,22 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS */ createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; + /** + * Constructs a new DapComm instance. + * Must be disposed. See `DapComm` documentation. + * + * @param session The Jupyter language runtime session. + * @param targetName The name of the comm target. + * @param debugType The type of debugger, as required by `vscode.DebugConfiguration.type`. + * @param debugName The name of the debugger, as required by `vscode.DebugConfiguration.name`. + * @returns A new `DapComm` instance. + */ + createDapComm( + targetName: string, + debugType: string, + debugName: string, + ): Promise; + /** * Method for emitting a message to the language server's Jupyter output * channel. @@ -233,16 +249,6 @@ export interface PositronSupervisorApi extends vscode.Disposable { sessionMetadata: positron.RuntimeSessionMetadata, dynState: positron.LanguageRuntimeDynState, ): Promise; - - /** - * The DAP comm class. - * - * Wraps a raw server comm (see `createServerComm()`) and provides an optional - * `handleMessage()` method for the standard DAP messages. - * - * Must be disposed. Disposing closes the comm if not already done. - */ - readonly DapComm: typeof DapComm; } /** Specific functionality implemented by runtimes */ @@ -361,29 +367,17 @@ export type CommBackendMessage = /** * A Debug Adapter Protocol (DAP) comm. * - * This wraps a raw comm that: + * This wraps a `Comm` that: * * - Implements the server protocol (see `createComm()` and * `JupyterLanguageRuntimeSession::createServerComm()`). * * - Optionally handles a standard set of DAP comm messages. + * + * Must be disposed when no longer in use or if `comm.receiver` is exhausted. + * Disposing the `DapComm` automatically disposes of the nested `Comm`. */ -export class DapComm { - /** - * Constructs a new DapComm instance. - * - * @param session The Jupyter language runtime session. - * @param targetName The name of the comm target. - * @param debugType The type of debugger, as required by `vscode.DebugConfiguration.type`. - * @param debugName The name of the debugger, as required by `vscode.DebugConfiguration.name`. - */ - constructor( - session: JupyterLanguageRuntimeSession, - targetName: string, - debugType: string, - debugName: string, - ); - +export interface DapComm { /** The `targetName` passed to the constructor. */ readonly targetName: string; @@ -407,7 +401,7 @@ export class DapComm { readonly serverPort?: number; /** - * Crate the raw server comm. + * Create the raw server comm. * * Calls `JupyterLanguageRuntimeSession::createServerComm()`. The backend * comm handling for `targetName` is expected to start a DAP server and From 2fbbf97b134eabb7a96488a005dade3af5743e56 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 21 Aug 2025 07:17:32 +0200 Subject: [PATCH 40/48] Make comm errors a tagged union Easier to match on from other extensions --- .../src/client/positron-supervisor.d.ts | 12 ++++++------ extensions/positron-r/src/positron-supervisor.d.ts | 12 ++++++------ .../positron-reticulate/src/positron-supervisor.d.ts | 12 ++++++------ .../positron-supervisor/src/positron-supervisor.d.ts | 12 ++++++------ 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/extensions/positron-python/src/client/positron-supervisor.d.ts b/extensions/positron-python/src/client/positron-supervisor.d.ts index 87f118d86f02..7f8b3d9407a2 100644 --- a/extensions/positron-python/src/client/positron-supervisor.d.ts +++ b/extensions/positron-python/src/client/positron-supervisor.d.ts @@ -314,23 +314,23 @@ export interface ReceiverChannel extends AsyncIterable, vscode.Disposable /** * Base class for communication errors. */ -export class CommError extends Error { - constructor(message: string, method?: string); +export interface CommError extends Error { + readonly name: 'CommError' | 'CommClosedError' | 'CommRpcError'; readonly method?: string; } /** * Error thrown when attempting to communicate through a closed channel. */ -export class CommClosedError extends CommError { - constructor(commId: string, method?: string); +export interface CommClosedError extends CommError { + readonly name: 'CommClosedError'; } /** * Error thrown for RPC-specific errors with error codes. */ -export class CommRpcError extends CommError { - constructor(message: string, code?: number, method?: string); +export interface CommRpcError extends CommError { + readonly name: 'CommRpcError'; readonly code: number; } diff --git a/extensions/positron-r/src/positron-supervisor.d.ts b/extensions/positron-r/src/positron-supervisor.d.ts index 87adb3bd8637..bdb5efb26bbb 100644 --- a/extensions/positron-r/src/positron-supervisor.d.ts +++ b/extensions/positron-r/src/positron-supervisor.d.ts @@ -322,23 +322,23 @@ export interface ReceiverChannel extends AsyncIterable, vscode.Disposable /** * Base class for communication errors. */ -export class CommError extends Error { - constructor(message: string, method?: string); +export interface CommError extends Error { + readonly name: 'CommError' | 'CommClosedError' | 'CommRpcError'; readonly method?: string; } /** * Error thrown when attempting to communicate through a closed channel. */ -export class CommClosedError extends CommError { - constructor(commId: string, method?: string); +export interface CommClosedError extends CommError { + readonly name: 'CommClosedError'; } /** * Error thrown for RPC-specific errors with error codes. */ -export class CommRpcError extends CommError { - constructor(message: string, code?: number, method?: string); +export interface CommRpcError extends CommError { + readonly name: 'CommRpcError'; readonly code: number; } diff --git a/extensions/positron-reticulate/src/positron-supervisor.d.ts b/extensions/positron-reticulate/src/positron-supervisor.d.ts index 87adb3bd8637..bdb5efb26bbb 100644 --- a/extensions/positron-reticulate/src/positron-supervisor.d.ts +++ b/extensions/positron-reticulate/src/positron-supervisor.d.ts @@ -322,23 +322,23 @@ export interface ReceiverChannel extends AsyncIterable, vscode.Disposable /** * Base class for communication errors. */ -export class CommError extends Error { - constructor(message: string, method?: string); +export interface CommError extends Error { + readonly name: 'CommError' | 'CommClosedError' | 'CommRpcError'; readonly method?: string; } /** * Error thrown when attempting to communicate through a closed channel. */ -export class CommClosedError extends CommError { - constructor(commId: string, method?: string); +export interface CommClosedError extends CommError { + readonly name: 'CommClosedError'; } /** * Error thrown for RPC-specific errors with error codes. */ -export class CommRpcError extends CommError { - constructor(message: string, code?: number, method?: string); +export interface CommRpcError extends CommError { + readonly name: 'CommRpcError'; readonly code: number; } diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index 87adb3bd8637..bdb5efb26bbb 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -322,23 +322,23 @@ export interface ReceiverChannel extends AsyncIterable, vscode.Disposable /** * Base class for communication errors. */ -export class CommError extends Error { - constructor(message: string, method?: string); +export interface CommError extends Error { + readonly name: 'CommError' | 'CommClosedError' | 'CommRpcError'; readonly method?: string; } /** * Error thrown when attempting to communicate through a closed channel. */ -export class CommClosedError extends CommError { - constructor(commId: string, method?: string); +export interface CommClosedError extends CommError { + readonly name: 'CommClosedError'; } /** * Error thrown for RPC-specific errors with error codes. */ -export class CommRpcError extends CommError { - constructor(message: string, code?: number, method?: string); +export interface CommRpcError extends CommError { + readonly name: 'CommRpcError'; readonly code: number; } From f8be3572b90d909784f59ed5d211a28e218d3628 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 21 Aug 2025 09:08:07 +0200 Subject: [PATCH 41/48] Include `id` field in nested JSON-RPC requests --- .../languageRuntime/common/positronBaseComm.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/services/languageRuntime/common/positronBaseComm.ts b/src/vs/workbench/services/languageRuntime/common/positronBaseComm.ts index 83f27f1b0631..64f607af58b8 100644 --- a/src/vs/workbench/services/languageRuntime/common/positronBaseComm.ts +++ b/src/vs/workbench/services/languageRuntime/common/positronBaseComm.ts @@ -7,6 +7,7 @@ import { IRuntimeClientInstance, RuntimeClientState, RuntimeClientStatus } from import { Event, Emitter } from '../../../../base/common/event.js'; import { Disposable } from '../../../../base/common/lifecycle.js'; import { ISettableObservable } from '../../../../base/common/observableInternal/base.js'; +import { generateUuid } from '../../../../base/common/uuid.js'; /** * An enum representing the set of JSON-RPC error codes. @@ -227,10 +228,18 @@ export class PositronBaseComm extends Disposable { rpcArgs[paramNames[i]] = paramValues[i]; } - // Form the request object + // Generate a unique ID for this message. + const id = generateUuid(); + + // Form the JSON-RPC message nested in our Jupyter `comm_msg`. Note that the + // `id` field is not used for matching requests at the Jupyter transport + // level. It only expresses that this is a request, not a notification, as + // required by the JSON-RPC spec. This allows comms at the other end to + // determine whether they should respond to the message. const request: any = { jsonrpc: '2.0', method: rpcName, + id }; // Amend params if we have any (methods which take no parameters From 2da874394bc3e30455f534baf09870776287958d Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 21 Aug 2025 09:32:49 +0200 Subject: [PATCH 42/48] Use JSON-RPC `id` field if present --- .../api/browser/positron/mainThreadLanguageRuntime.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/api/browser/positron/mainThreadLanguageRuntime.ts b/src/vs/workbench/api/browser/positron/mainThreadLanguageRuntime.ts index 876d33a4e2f8..07025d140c6c 100644 --- a/src/vs/workbench/api/browser/positron/mainThreadLanguageRuntime.ts +++ b/src/vs/workbench/api/browser/positron/mainThreadLanguageRuntime.ts @@ -1174,7 +1174,14 @@ class ExtHostRuntimeClientInstance */ performRpcWithBuffers(request: Input, timeout: number | undefined, responseKeys: Array = []): Promise> { // Generate a unique ID for this message. - const messageId = generateUuid(); + let messageId; + if ((request as any)?.id) { + // If the request already has an id field, use it as id. This is typically + // the case with nested JSON-RPC messages. + messageId = (request as any).id; + } else { + messageId = generateUuid(); + } // Add the promise to the list of pending RPCs. const pending = new PendingRpc(responseKeys); From dca18425189c0c9a15093be6609df00795006058 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 21 Aug 2025 09:35:00 +0200 Subject: [PATCH 43/48] Bump Ark --- extensions/positron-r/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/positron-r/package.json b/extensions/positron-r/package.json index 9a4ed8aec605..103ca1e39e0b 100644 --- a/extensions/positron-r/package.json +++ b/extensions/positron-r/package.json @@ -974,7 +974,7 @@ }, "positron": { "binaryDependencies": { - "ark": "0.1.209" + "ark": "posit-dev/ark@task/dap-comm" }, "minimumRVersion": "4.2.0", "minimumRenvVersion": "1.0.9" From 23f3d80042de424df9ca0f671cb9e8c4dcad545f Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 21 Aug 2025 12:29:08 +0200 Subject: [PATCH 44/48] Rename `RawComm` to `Comm` --- .../src/client/positron-supervisor.d.ts | 22 ++++++------- extensions/positron-r/src/ark-comm.ts | 6 ++-- .../positron-r/src/positron-supervisor.d.ts | 22 ++++++------- .../positron-r/src/test/ark-comm.test.ts | 8 ++--- .../src/positron-supervisor.d.ts | 22 ++++++------- .../src/{RawComm.ts => Comm.ts} | 2 +- extensions/positron-supervisor/src/DapComm.ts | 6 ++-- .../src/KallichoreSession.ts | 32 +++++++++---------- .../src/positron-supervisor.d.ts | 22 ++++++------- 9 files changed, 71 insertions(+), 71 deletions(-) rename extensions/positron-supervisor/src/{RawComm.ts => Comm.ts} (98%) diff --git a/extensions/positron-python/src/client/positron-supervisor.d.ts b/extensions/positron-python/src/client/positron-supervisor.d.ts index 7f8b3d9407a2..f621132bf554 100644 --- a/extensions/positron-python/src/client/positron-supervisor.d.ts +++ b/extensions/positron-python/src/client/positron-supervisor.d.ts @@ -99,7 +99,7 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS createPositronLspClientId(): string; /** - * Start a raw comm for communication between frontend and backend. + * Start a comm for communication between frontend and backend. * * Unlike Positron clients, this kind of comm is private to the calling * extension and its kernel. They open a direct line of communication that @@ -121,12 +121,12 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * @param target_name Comm type, also used to generate comm identifier. * @param params Optionally, additional parameters included in `comm_open`. */ - createComm(target_name: string, params?: Record): Promise; + createComm(target_name: string, params?: Record): Promise; /** - * Create a raw server comm. + * Create a server comm. * - * Server comms are a special type of raw comms (see `createComm()`) that + * Server comms are a special type of comms (see `createComm()`) that * wrap a TCP server (e.g. an LSP or DAP server). The backend is expected to * handle `comm_open` messages for `targetName` comms in the following way: * @@ -141,10 +141,10 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * * @param targetName The name of the comm target * @param host The IP address or host name for the server - * @returns A promise that resolves to a tuple of [RawComm, port number] + * @returns A promise that resolves to a tuple of [Comm, port number] * once the server has been started on the backend side. */ - createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; + createServerComm(targetName: string, host: string): Promise<[Comm, number]>; /** * Constructs a new DapComm instance. @@ -255,7 +255,7 @@ export interface JupyterKernelExtra { } /** - * Raw comm unmanaged by Positron. + * Comm between an extension and its kernel. * * This type of comm is not mapped to a Positron client. It lives entirely in * the extension space and allows a direct line of communication between an @@ -265,7 +265,7 @@ export interface JupyterKernelExtra { * it. If the comm has not already been closed by the kernel, a client-initiated * `comm_close` message is emitted to clean the comm on the backend side. */ -export interface RawComm { +export interface Comm { /** The comm ID. */ id: string; @@ -380,11 +380,11 @@ export interface DapComm { readonly debugName: string; /** - * The raw comm for the DAP. + * The comm for the DAP. * Use it to receive messages or make notifications and requests. * Defined after `createServerComm()` has been called. */ - readonly comm?: RawComm; + readonly comm?: Comm; /** * The port on which the DAP server is listening. @@ -393,7 +393,7 @@ export interface DapComm { readonly serverPort?: number; /** - * Create the raw server comm. + * Create the server comm. * * Calls `JupyterLanguageRuntimeSession::createServerComm()`. The backend * comm handling for `targetName` is expected to start a DAP server and diff --git a/extensions/positron-r/src/ark-comm.ts b/extensions/positron-r/src/ark-comm.ts index e23046edbd80..faa3d5f233b1 100644 --- a/extensions/positron-r/src/ark-comm.ts +++ b/extensions/positron-r/src/ark-comm.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as vscode from 'vscode'; -import { JupyterLanguageRuntimeSession, RawComm } from './positron-supervisor'; +import { JupyterLanguageRuntimeSession, Comm } from './positron-supervisor'; import { LOGGER } from './extension'; /** @@ -14,11 +14,11 @@ import { LOGGER } from './extension'; export class ArkComm implements vscode.Disposable { readonly targetName: string = 'ark'; - public get comm(): RawComm | undefined { + public get comm(): Comm | undefined { return this._comm; } - private _comm?: RawComm; + private _comm?: Comm; constructor( private session: JupyterLanguageRuntimeSession, diff --git a/extensions/positron-r/src/positron-supervisor.d.ts b/extensions/positron-r/src/positron-supervisor.d.ts index bdb5efb26bbb..1da562c35a3e 100644 --- a/extensions/positron-r/src/positron-supervisor.d.ts +++ b/extensions/positron-r/src/positron-supervisor.d.ts @@ -99,7 +99,7 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS createPositronLspClientId(): string; /** - * Start a raw comm for communication between frontend and backend. + * Start a comm for communication between frontend and backend. * * Unlike Positron clients, this kind of comm is private to the calling * extension and its kernel. They open a direct line of communication that @@ -124,12 +124,12 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS createComm( target_name: string, params?: Record, - ): Promise; + ): Promise; /** - * Create a raw server comm. + * Create a server comm. * - * Server comms are a special type of raw comms (see `createComm()`) that + * Server comms are a special type of comms (see `createComm()`) that * wrap a TCP server (e.g. an LSP or DAP server). The backend is expected to * handle `comm_open` messages for `targetName` comms in the following way: * @@ -144,10 +144,10 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * * @param targetName The name of the comm target * @param host The IP address or host name for the server - * @returns A promise that resolves to a tuple of [RawComm, port number] + * @returns A promise that resolves to a tuple of [Comm, port number] * once the server has been started on the backend side. */ - createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; + createServerComm(targetName: string, host: string): Promise<[Comm, number]>; /** * Constructs a new DapComm instance. @@ -263,7 +263,7 @@ export interface JupyterKernelExtra { } /** - * Raw comm unmanaged by Positron. + * Comm between an extension and its kernel. * * This type of comm is not mapped to a Positron client. It lives entirely in * the extension space and allows a direct line of communication between an @@ -273,7 +273,7 @@ export interface JupyterKernelExtra { * it. If the comm has not already been closed by the kernel, a client-initiated * `comm_close` message is emitted to clean the comm on the backend side. */ -export interface RawComm { +export interface Comm { /** The comm ID. */ id: string; @@ -388,11 +388,11 @@ export interface DapComm { readonly debugName: string; /** - * The raw comm for the DAP. + * The comm for the DAP. * Use it to receive messages or make notifications and requests. * Defined after `createServerComm()` has been called. */ - readonly comm?: RawComm; + readonly comm?: Comm; /** * The port on which the DAP server is listening. @@ -401,7 +401,7 @@ export interface DapComm { readonly serverPort?: number; /** - * Create the raw server comm. + * Create the server comm. * * Calls `JupyterLanguageRuntimeSession::createServerComm()`. The backend * comm handling for `targetName` is expected to start a DAP server and diff --git a/extensions/positron-r/src/test/ark-comm.test.ts b/extensions/positron-r/src/test/ark-comm.test.ts index 0500ee136421..8e1646c1af5e 100644 --- a/extensions/positron-r/src/test/ark-comm.test.ts +++ b/extensions/positron-r/src/test/ark-comm.test.ts @@ -7,13 +7,13 @@ import * as assert from 'assert'; import * as vscode from 'vscode'; import * as testKit from './kit'; import { RSession } from '../session'; -import { RawComm, CommBackendMessage } from '../positron-supervisor'; +import { Comm, CommBackendMessage } from '../positron-supervisor'; import { whenTimeout } from '../util'; suite('ArkComm', () => { let session: RSession; let sesDisposable: vscode.Disposable; - let comm: RawComm; + let comm: Comm; suiteSetup(async () => { const [ses, disposable] = await testKit.startR(); @@ -78,7 +78,7 @@ suite('ArkComm', () => { }); }); -async function assertNextMessage(comm: RawComm): Promise { +async function assertNextMessage(comm: Comm): Promise { const result = await Promise.race([ comm.receiver.next(), whenTimeout(5000, () => assert.fail(`Timeout while expecting comm message on ${comm.id}`)), @@ -88,7 +88,7 @@ async function assertNextMessage(comm: RawComm): Promise { return result.value; } -async function assertRequest(comm: RawComm, method: string, params?: Record): Promise { +async function assertRequest(comm: Comm, method: string, params?: Record): Promise { return await Promise.race([ comm.request(method, params), whenTimeout(5000, () => assert.fail(`Timeout while expecting comm reply on ${comm.id}`)), diff --git a/extensions/positron-reticulate/src/positron-supervisor.d.ts b/extensions/positron-reticulate/src/positron-supervisor.d.ts index bdb5efb26bbb..1da562c35a3e 100644 --- a/extensions/positron-reticulate/src/positron-supervisor.d.ts +++ b/extensions/positron-reticulate/src/positron-supervisor.d.ts @@ -99,7 +99,7 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS createPositronLspClientId(): string; /** - * Start a raw comm for communication between frontend and backend. + * Start a comm for communication between frontend and backend. * * Unlike Positron clients, this kind of comm is private to the calling * extension and its kernel. They open a direct line of communication that @@ -124,12 +124,12 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS createComm( target_name: string, params?: Record, - ): Promise; + ): Promise; /** - * Create a raw server comm. + * Create a server comm. * - * Server comms are a special type of raw comms (see `createComm()`) that + * Server comms are a special type of comms (see `createComm()`) that * wrap a TCP server (e.g. an LSP or DAP server). The backend is expected to * handle `comm_open` messages for `targetName` comms in the following way: * @@ -144,10 +144,10 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * * @param targetName The name of the comm target * @param host The IP address or host name for the server - * @returns A promise that resolves to a tuple of [RawComm, port number] + * @returns A promise that resolves to a tuple of [Comm, port number] * once the server has been started on the backend side. */ - createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; + createServerComm(targetName: string, host: string): Promise<[Comm, number]>; /** * Constructs a new DapComm instance. @@ -263,7 +263,7 @@ export interface JupyterKernelExtra { } /** - * Raw comm unmanaged by Positron. + * Comm between an extension and its kernel. * * This type of comm is not mapped to a Positron client. It lives entirely in * the extension space and allows a direct line of communication between an @@ -273,7 +273,7 @@ export interface JupyterKernelExtra { * it. If the comm has not already been closed by the kernel, a client-initiated * `comm_close` message is emitted to clean the comm on the backend side. */ -export interface RawComm { +export interface Comm { /** The comm ID. */ id: string; @@ -388,11 +388,11 @@ export interface DapComm { readonly debugName: string; /** - * The raw comm for the DAP. + * The comm for the DAP. * Use it to receive messages or make notifications and requests. * Defined after `createServerComm()` has been called. */ - readonly comm?: RawComm; + readonly comm?: Comm; /** * The port on which the DAP server is listening. @@ -401,7 +401,7 @@ export interface DapComm { readonly serverPort?: number; /** - * Create the raw server comm. + * Create the server comm. * * Calls `JupyterLanguageRuntimeSession::createServerComm()`. The backend * comm handling for `targetName` is expected to start a DAP server and diff --git a/extensions/positron-supervisor/src/RawComm.ts b/extensions/positron-supervisor/src/Comm.ts similarity index 98% rename from extensions/positron-supervisor/src/RawComm.ts rename to extensions/positron-supervisor/src/Comm.ts index 82a78b7c6ada..99723cfc4290 100644 --- a/extensions/positron-supervisor/src/RawComm.ts +++ b/extensions/positron-supervisor/src/Comm.ts @@ -40,7 +40,7 @@ export class CommRpcError extends CommError { } } -export class RawCommImpl implements vscode.Disposable { +export class CommImpl implements vscode.Disposable { private readonly disposables: vscode.Disposable[] = []; private closed = false; diff --git a/extensions/positron-supervisor/src/DapComm.ts b/extensions/positron-supervisor/src/DapComm.ts index a1f6acfddca8..8834e18031e0 100644 --- a/extensions/positron-supervisor/src/DapComm.ts +++ b/extensions/positron-supervisor/src/DapComm.ts @@ -5,21 +5,21 @@ import * as vscode from 'vscode'; import * as positron from 'positron'; -import { JupyterLanguageRuntimeSession, RawComm } from './positron-supervisor'; +import { JupyterLanguageRuntimeSession, Comm } from './positron-supervisor'; /** * A Debug Adapter Protocol (DAP) comm. * See `positron-supervisor.d.ts` for documentation. */ export class DapComm { - public get comm(): RawComm | undefined { + public get comm(): Comm | undefined { return this._comm; } public get port(): number | undefined { return this._port; } - private _comm?: RawComm; + private _comm?: Comm; private _port?: number; // Message counter used for creating unique message IDs diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 90fcc5a22e4d..328e7d0b3f7b 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -8,7 +8,7 @@ import * as positron from 'positron'; import * as os from 'os'; import * as path from 'path'; import * as fs from 'fs'; -import { CommBackendMessage, JupyterKernelExtra, JupyterKernelSpec, JupyterLanguageRuntimeSession, JupyterSession, RawComm } from './positron-supervisor'; +import { CommBackendMessage, JupyterKernelExtra, JupyterKernelSpec, JupyterLanguageRuntimeSession, JupyterSession, Comm } from './positron-supervisor'; import { ActiveSession, ConnectionInfo, DefaultApi, HttpError, InterruptMode, NewSession, RestartSession, Status, VarAction, VarActionType } from './kcclient/api'; import { JupyterMessage } from './jupyter/JupyterMessage'; import { JupyterRequest } from './jupyter/JupyterRequest'; @@ -41,7 +41,7 @@ import { AdoptedSession } from './AdoptedSession'; import { DebugRequest } from './jupyter/DebugRequest'; import { JupyterMessageType } from './jupyter/JupyterMessageType.js'; import { JupyterCommClose } from './jupyter/JupyterCommClose'; -import { CommBackendRequest, CommRpcMessage, RawCommImpl } from './RawComm'; +import { CommBackendRequest, CommRpcMessage, CommImpl } from './Comm'; import { channel, Sender } from './Channel'; import { DapComm } from './DapComm'; @@ -151,7 +151,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { private readonly _clients: Map = new Map(); /** A map of active comms unmanaged by Positron */ - private readonly _rawComms: Map]> = new Map(); + private readonly _comms: Map]> = new Map(); /** The kernel's log file, if any. */ private _kernelLogFile: string | undefined; @@ -464,7 +464,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { } /** Create a raw server comm. See `positron-supervisor.d.ts` for documentation. */ - async createServerComm(target_name: string, host: string): Promise<[RawComm, number]> { + async createServerComm(target_name: string, host: string): Promise<[Comm, number]> { this.log(`Starting server comm '${target_name}' for ${host}`); const comm = await this.createComm(target_name, { ip_address: host }); @@ -724,19 +724,19 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { async createComm( target_name: string, params: Record = {}, - ): Promise { + ): Promise { const id = `extension-comm-${target_name}-${this.runtimeMetadata.languageId}-${createUniqueId()}`; const [tx, rx] = channel(); - const comm = new RawCommImpl(id, this, rx); - this._rawComms.set(id, [comm, tx]); + const comm = new CommImpl(id, this, rx); + this._comms.set(id, [comm, tx]); // Disposal handler that allows extension to initiate close comm comm.register({ dispose: () => { // If already deleted, it means a `comm_close` from the backend was // received and we don't need to send one. - if (this._rawComms.delete(id)) { + if (this._comms.delete(id)) { comm.closeAndNotify(); } } @@ -750,7 +750,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { const commOpen = new CommOpenCommand(msg); await this.sendCommand(commOpen); - return comm as RawComm; + return comm as Comm; } /** Create DAP comm. See `positron-supervisor.d.ts` for documentation. */ @@ -822,7 +822,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { // Unwrap the comm info and add it to the result for (const key in comms) { // Don't list as client if this is an unmanaged comm - if (this._rawComms.has(key)) { + if (this._comms.has(key)) { continue; } @@ -1790,12 +1790,12 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { this._clients.clear(); // Close all raw comms - for (const [comm, tx] of this._rawComms.values()) { + for (const [comm, tx] of this._comms.values()) { // Don't dispose of comm, this resource is owned by caller of `createComm()`. comm.close(); tx.dispose(); } - this._rawComms.clear(); + this._comms.clear(); // Clear any starting comms this._startingComms.forEach((promise) => { @@ -1915,7 +1915,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { // promise. if (msg.header.msg_type === 'comm_msg') { const commMsg = msg.content as JupyterCommMsg; - if (this._rawComms.has(commMsg.comm_id)) { + if (this._comms.has(commMsg.comm_id)) { return; } } @@ -1947,12 +1947,12 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { switch (msg.header.msg_type) { case 'comm_close': { const closeMsg = msg.content as JupyterCommClose; - const commHandle = this._rawComms.get(closeMsg.comm_id); + const commHandle = this._comms.get(closeMsg.comm_id); if (commHandle) { // Delete first, this prevents the channel disposable from sending a // `comm_close` back - this._rawComms.delete(closeMsg.comm_id); + this._comms.delete(closeMsg.comm_id); const [comm, _] = commHandle; comm.close(); @@ -1964,7 +1964,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { case 'comm_msg': { const commMsg = msg.content as JupyterCommMsg; - const commHandle = this._rawComms.get(commMsg.comm_id); + const commHandle = this._comms.get(commMsg.comm_id); if (commHandle) { const [_, tx] = commHandle; diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index bdb5efb26bbb..1da562c35a3e 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -99,7 +99,7 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS createPositronLspClientId(): string; /** - * Start a raw comm for communication between frontend and backend. + * Start a comm for communication between frontend and backend. * * Unlike Positron clients, this kind of comm is private to the calling * extension and its kernel. They open a direct line of communication that @@ -124,12 +124,12 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS createComm( target_name: string, params?: Record, - ): Promise; + ): Promise; /** - * Create a raw server comm. + * Create a server comm. * - * Server comms are a special type of raw comms (see `createComm()`) that + * Server comms are a special type of comms (see `createComm()`) that * wrap a TCP server (e.g. an LSP or DAP server). The backend is expected to * handle `comm_open` messages for `targetName` comms in the following way: * @@ -144,10 +144,10 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * * @param targetName The name of the comm target * @param host The IP address or host name for the server - * @returns A promise that resolves to a tuple of [RawComm, port number] + * @returns A promise that resolves to a tuple of [Comm, port number] * once the server has been started on the backend side. */ - createServerComm(targetName: string, host: string): Promise<[RawComm, number]>; + createServerComm(targetName: string, host: string): Promise<[Comm, number]>; /** * Constructs a new DapComm instance. @@ -263,7 +263,7 @@ export interface JupyterKernelExtra { } /** - * Raw comm unmanaged by Positron. + * Comm between an extension and its kernel. * * This type of comm is not mapped to a Positron client. It lives entirely in * the extension space and allows a direct line of communication between an @@ -273,7 +273,7 @@ export interface JupyterKernelExtra { * it. If the comm has not already been closed by the kernel, a client-initiated * `comm_close` message is emitted to clean the comm on the backend side. */ -export interface RawComm { +export interface Comm { /** The comm ID. */ id: string; @@ -388,11 +388,11 @@ export interface DapComm { readonly debugName: string; /** - * The raw comm for the DAP. + * The comm for the DAP. * Use it to receive messages or make notifications and requests. * Defined after `createServerComm()` has been called. */ - readonly comm?: RawComm; + readonly comm?: Comm; /** * The port on which the DAP server is listening. @@ -401,7 +401,7 @@ export interface DapComm { readonly serverPort?: number; /** - * Create the raw server comm. + * Create the server comm. * * Calls `JupyterLanguageRuntimeSession::createServerComm()`. The backend * comm handling for `targetName` is expected to start a DAP server and From 37726668a5c24d7154f3f4cca37a6342e272a2ac Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 21 Aug 2025 12:37:30 +0200 Subject: [PATCH 45/48] No longer `host` but `ip_address` --- extensions/positron-supervisor/src/KallichoreSession.ts | 8 ++++---- .../positron-supervisor/src/positron-supervisor.d.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 328e7d0b3f7b..4ecfbcbd2294 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -464,9 +464,9 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { } /** Create a raw server comm. See `positron-supervisor.d.ts` for documentation. */ - async createServerComm(target_name: string, host: string): Promise<[Comm, number]> { - this.log(`Starting server comm '${target_name}' for ${host}`); - const comm = await this.createComm(target_name, { ip_address: host }); + async createServerComm(target_name: string, ip_address: string): Promise<[Comm, number]> { + this.log(`Starting server comm '${target_name}' for ${ip_address}`); + const comm = await this.createComm(target_name, { ip_address }); const result = await comm.receiver.next(); @@ -490,7 +490,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { throw new Error('`server_started` message doesn\'t include a port'); } - this.log(`Started server comm '${target_name}' for ${host} on port ${port}`); + this.log(`Started server comm '${target_name}' for ${ip_address} on port ${port}`); return [comm, port]; } diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index 1da562c35a3e..2e7bd04eb099 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -143,11 +143,11 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * includes a field `port`. * * @param targetName The name of the comm target - * @param host The IP address or host name for the server + * @param ip_address The IP address to which the server should bind to. * @returns A promise that resolves to a tuple of [Comm, port number] * once the server has been started on the backend side. */ - createServerComm(targetName: string, host: string): Promise<[Comm, number]>; + createServerComm(targetName: string, ip_address: string): Promise<[Comm, number]>; /** * Constructs a new DapComm instance. From 5d3ed25dcf2c106b016df2636adacb2ab00d637d Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Thu, 28 Aug 2025 12:03:46 +0200 Subject: [PATCH 46/48] Remove `createComm()` from public interface of `DapComm` --- .../src/client/positron-supervisor.d.ts | 17 ++--------------- .../positron-r/src/positron-supervisor.d.ts | 17 ++--------------- .../src/positron-supervisor.d.ts | 17 ++--------------- .../src/positron-supervisor.d.ts | 13 ------------- 4 files changed, 6 insertions(+), 58 deletions(-) diff --git a/extensions/positron-python/src/client/positron-supervisor.d.ts b/extensions/positron-python/src/client/positron-supervisor.d.ts index f621132bf554..380b2ab8b354 100644 --- a/extensions/positron-python/src/client/positron-supervisor.d.ts +++ b/extensions/positron-python/src/client/positron-supervisor.d.ts @@ -140,11 +140,11 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * includes a field `port`. * * @param targetName The name of the comm target - * @param host The IP address or host name for the server + * @param ip_address The IP address to which the server should bind to. * @returns A promise that resolves to a tuple of [Comm, port number] * once the server has been started on the backend side. */ - createServerComm(targetName: string, host: string): Promise<[Comm, number]>; + createServerComm(targetName: string, ip_address: string): Promise<[Comm, number]>; /** * Constructs a new DapComm instance. @@ -392,19 +392,6 @@ export interface DapComm { */ readonly serverPort?: number; - /** - * Create the server comm. - * - * Calls `JupyterLanguageRuntimeSession::createServerComm()`. The backend - * comm handling for `targetName` is expected to start a DAP server and - * communicate the port as part of the handshake. - * - * Once resolved: - * - The DAP is ready to accept connections on the backend side. - * - `comm` and `serverPort` are defined. - */ - createComm(): Promise; - /** * Handle a message received via `this.comm.receiver`. * diff --git a/extensions/positron-r/src/positron-supervisor.d.ts b/extensions/positron-r/src/positron-supervisor.d.ts index 1da562c35a3e..5a36ebbb69d4 100644 --- a/extensions/positron-r/src/positron-supervisor.d.ts +++ b/extensions/positron-r/src/positron-supervisor.d.ts @@ -143,11 +143,11 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * includes a field `port`. * * @param targetName The name of the comm target - * @param host The IP address or host name for the server + * @param ip_address The IP address to which the server should bind to. * @returns A promise that resolves to a tuple of [Comm, port number] * once the server has been started on the backend side. */ - createServerComm(targetName: string, host: string): Promise<[Comm, number]>; + createServerComm(targetName: string, ip_address: string): Promise<[Comm, number]>; /** * Constructs a new DapComm instance. @@ -400,19 +400,6 @@ export interface DapComm { */ readonly serverPort?: number; - /** - * Create the server comm. - * - * Calls `JupyterLanguageRuntimeSession::createServerComm()`. The backend - * comm handling for `targetName` is expected to start a DAP server and - * communicate the port as part of the handshake. - * - * Once resolved: - * - The DAP is ready to accept connections on the backend side. - * - `comm` and `serverPort` are defined. - */ - createComm(): Promise; - /** * Handle a message received via `this.comm.receiver`. * diff --git a/extensions/positron-reticulate/src/positron-supervisor.d.ts b/extensions/positron-reticulate/src/positron-supervisor.d.ts index 1da562c35a3e..5a36ebbb69d4 100644 --- a/extensions/positron-reticulate/src/positron-supervisor.d.ts +++ b/extensions/positron-reticulate/src/positron-supervisor.d.ts @@ -143,11 +143,11 @@ export interface JupyterLanguageRuntimeSession extends positron.LanguageRuntimeS * includes a field `port`. * * @param targetName The name of the comm target - * @param host The IP address or host name for the server + * @param ip_address The IP address to which the server should bind to. * @returns A promise that resolves to a tuple of [Comm, port number] * once the server has been started on the backend side. */ - createServerComm(targetName: string, host: string): Promise<[Comm, number]>; + createServerComm(targetName: string, ip_address: string): Promise<[Comm, number]>; /** * Constructs a new DapComm instance. @@ -400,19 +400,6 @@ export interface DapComm { */ readonly serverPort?: number; - /** - * Create the server comm. - * - * Calls `JupyterLanguageRuntimeSession::createServerComm()`. The backend - * comm handling for `targetName` is expected to start a DAP server and - * communicate the port as part of the handshake. - * - * Once resolved: - * - The DAP is ready to accept connections on the backend side. - * - `comm` and `serverPort` are defined. - */ - createComm(): Promise; - /** * Handle a message received via `this.comm.receiver`. * diff --git a/extensions/positron-supervisor/src/positron-supervisor.d.ts b/extensions/positron-supervisor/src/positron-supervisor.d.ts index 2e7bd04eb099..5a36ebbb69d4 100644 --- a/extensions/positron-supervisor/src/positron-supervisor.d.ts +++ b/extensions/positron-supervisor/src/positron-supervisor.d.ts @@ -400,19 +400,6 @@ export interface DapComm { */ readonly serverPort?: number; - /** - * Create the server comm. - * - * Calls `JupyterLanguageRuntimeSession::createServerComm()`. The backend - * comm handling for `targetName` is expected to start a DAP server and - * communicate the port as part of the handshake. - * - * Once resolved: - * - The DAP is ready to accept connections on the backend side. - * - `comm` and `serverPort` are defined. - */ - createComm(): Promise; - /** * Handle a message received via `this.comm.receiver`. * From a8f46dc144041b9df6bb6ef7807b838940292781 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Sat, 20 Sep 2025 08:39:20 -0400 Subject: [PATCH 47/48] Address code review --- extensions/positron-supervisor/src/Channel.ts | 8 ++++---- extensions/positron-supervisor/src/Comm.ts | 3 +++ extensions/positron-supervisor/src/KallichoreSession.ts | 1 - 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/extensions/positron-supervisor/src/Channel.ts b/extensions/positron-supervisor/src/Channel.ts index fb77b538220d..641b6894c7eb 100644 --- a/extensions/positron-supervisor/src/Channel.ts +++ b/extensions/positron-supervisor/src/Channel.ts @@ -66,7 +66,7 @@ export class Sender implements vscode.Disposable { * Channel receiver (rx). Async-iterable to receive values from the channel. */ export class Receiver implements AsyncIterable, AsyncIterator, vscode.Disposable { - private i = 0; + private yieldCount = 0; private disposables: vscode.Disposable[] = []; constructor(private state: ChannelState) { } @@ -77,15 +77,15 @@ export class Receiver implements AsyncIterable, AsyncIterator, vscode.D async next(): Promise> { if (this.state.queue.length > 0) { - ++this.i; + ++this.yieldCount; // Get the value from queue first, before any potential await const value = this.state.queue.shift()!; // Yield regularly to event loop to avoid starvation. Sends are // synchronous and handlers might be synchronous as well. - if (this.i > YIELD_THRESHOLD) { - this.i = 0; + if (this.yieldCount > YIELD_THRESHOLD) { + this.yieldCount = 0; await delay(0); } diff --git a/extensions/positron-supervisor/src/Comm.ts b/extensions/positron-supervisor/src/Comm.ts index 99723cfc4290..bb8bccae8e50 100644 --- a/extensions/positron-supervisor/src/Comm.ts +++ b/extensions/positron-supervisor/src/Comm.ts @@ -125,6 +125,9 @@ export class CommImpl implements vscode.Disposable { for (const disposable of this.disposables) { disposable.dispose(); } + + // Clear array to make method idempotent + this.disposables.length = 0; } register(disposable: vscode.Disposable) { diff --git a/extensions/positron-supervisor/src/KallichoreSession.ts b/extensions/positron-supervisor/src/KallichoreSession.ts index 4ecfbcbd2294..54abc4cf6d55 100644 --- a/extensions/positron-supervisor/src/KallichoreSession.ts +++ b/extensions/positron-supervisor/src/KallichoreSession.ts @@ -831,7 +831,6 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession { result[key] = target; // If we don't have a comm object for this comm, create one if (!this._clients.has(key)) { - // Should this be renamed to Client? this._clients.set(key, new Client(key, target)); } From a6abaab6bd60584328fc26a8fad677977074e21a Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Sat, 20 Sep 2025 09:06:17 -0400 Subject: [PATCH 48/48] Bump Ark to 0.1.210 --- extensions/positron-r/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/positron-r/package.json b/extensions/positron-r/package.json index 103ca1e39e0b..b82cf90a46f3 100644 --- a/extensions/positron-r/package.json +++ b/extensions/positron-r/package.json @@ -974,7 +974,7 @@ }, "positron": { "binaryDependencies": { - "ark": "posit-dev/ark@task/dap-comm" + "ark": "0.1.210" }, "minimumRVersion": "4.2.0", "minimumRenvVersion": "1.0.9"