Skip to content

Commit c90f4df

Browse files
authored
Properly pass through buffers on comm_open events (#7228)
1 parent 268f89e commit c90f4df

File tree

7 files changed

+52
-7
lines changed

7 files changed

+52
-7
lines changed

extensions/positron-ipywidgets/renderer/src/manager.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*---------------------------------------------------------------------------------------------
2-
* Copyright (C) 2024 Posit Software, PBC. All rights reserved.
2+
* Copyright (C) 2024-2025 Posit Software, PBC. All rights reserved.
33
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
44
*--------------------------------------------------------------------------------------------*/
55

@@ -134,6 +134,9 @@ export class PositronWidgetManager extends ManagerBase implements base.IWidgetMa
134134
target_name: message.target_name,
135135
data: message.data as JSONObject,
136136
},
137+
// Sometimes these buffers are not aligned, so we need to convert them to Uint8Arrays to avoid mangled data.
138+
// This is handled by the Comm class for subsequent messages.
139+
buffers: message.buffers?.map(buffer => new Uint8Array(buffer.buffer)),
137140
// This is expected to at least contain the backend widget protocol 'version', which
138141
// should match the frontend version.
139142
metadata: message.metadata as JSONObject,

extensions/positron-supervisor/src/KallichoreSession.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1526,7 +1526,7 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession {
15261526
*/
15271527
handleJupyterMessage(data: any) {
15281528
// Deserialize the message buffers from base64, if any
1529-
if (data.buffers) {
1529+
if (data.buffers?.length > 0) {
15301530
data.buffers = data.buffers.map((b: string) => {
15311531
return Buffer.from(b, 'base64');
15321532
});

extensions/positron-supervisor/src/RuntimeMessageEmitter.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*---------------------------------------------------------------------------------------------
2-
* Copyright (C) 2023-2024 Posit Software, PBC. All rights reserved.
2+
* Copyright (C) 2023-2025 Posit Software, PBC. All rights reserved.
33
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
44
*--------------------------------------------------------------------------------------------*/
55

@@ -184,6 +184,7 @@ export class RuntimeMessageEmitter {
184184
target_name: data.target_name,
185185
data: data.data,
186186
metadata: message.metadata,
187+
buffers: message.buffers,
187188
} as positron.LanguageRuntimeCommOpen);
188189
}
189190

src/vs/workbench/contrib/positronIPyWidgets/browser/positronIPyWidgetsService.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*---------------------------------------------------------------------------------------------
2-
* Copyright (C) 2023-2024 Posit Software, PBC. All rights reserved.
2+
* Copyright (C) 2023-2025 Posit Software, PBC. All rights reserved.
33
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
44
*--------------------------------------------------------------------------------------------*/
55
import { Disposable, DisposableStore, toDisposable } from '../../../../base/common/lifecycle.js';
@@ -323,6 +323,7 @@ export class IPyWidgetsInstance extends Disposable {
323323
target_name: client.getClientType(),
324324
data: message.data,
325325
metadata: message.metadata,
326+
buffers: message.buffers,
326327
});
327328
}));
328329

src/vs/workbench/contrib/positronIPyWidgets/test/browser/positronIPyWidgetsService.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { PositronTestServiceAccessor, positronWorkbenchInstantiationService } fr
2727
import { INotebookRendererInfo, INotebookStaticPreloadInfo } from '../../../notebook/common/notebookCommon.js';
2828
import { NotebookOutputRendererInfo } from '../../../notebook/common/notebookOutputRenderer.js';
2929
import { ExtensionIdentifier } from '../../../../../platform/extensions/common/extensions.js';
30+
import { VSBuffer } from '../../../../../base/common/buffer.js';
3031

3132
class TestNotebookService implements Partial<INotebookService> {
3233
getRenderers(): INotebookRendererInfo[] {
@@ -246,6 +247,40 @@ suite('Positron - IPyWidgetsInstance constructor', () => {
246247
assert.deepStrictEqual(messaging.messagesToWebview, [{ type: 'initialize_result' }]);
247248
});
248249

250+
test('forwards comm_open with buffers when client is created via session.createClient', async () => {
251+
// Create instance and clear initial messages
252+
await createIPyWidgetsInstance();
253+
messaging.messagesToWebview.length = 0; // Clear initialize_result message
254+
255+
// Make some sample client data
256+
const clientId = 'new-widget-client-id';
257+
const clientType = RuntimeClientType.IPyWidget;
258+
const messageData = { initial_state: 'some_value' };
259+
const messageMetadata = { source: 'test' };
260+
const messageBuffers = [
261+
VSBuffer.wrap(new Uint8Array([1, 2, 3])),
262+
VSBuffer.wrap(new Uint8Array([4, 5]))
263+
];
264+
265+
// Act: Call session.createClient to simulate client creation and trigger the event
266+
await session.createClient(
267+
clientType,
268+
messageData,
269+
messageMetadata,
270+
clientId,
271+
messageBuffers
272+
);
273+
await timeout(0);
274+
275+
// Sanity check: only one message should be sent.
276+
assert.strictEqual(messaging.messagesToWebview.length, 1, 'Expected exactly one message (comm_open) to be sent to the webview');
277+
const sentMessage = messaging.messagesToWebview[0];
278+
279+
// The message should be comm_open and contain the correct buffers.
280+
assert.strictEqual(sentMessage.type, 'comm_open', 'Expected message type to be comm_open');
281+
assert.deepStrictEqual(sentMessage.buffers, messageBuffers, 'Expected buffers to be passed correctly in the comm_open message');
282+
});
283+
249284
test('initialized session, one ipywidget client', async () => {
250285
const client = await session.createClient(RuntimeClientType.IPyWidget, {}, {}, 'test-client-id');
251286

@@ -365,6 +400,7 @@ suite('Positron - IPyWidgetsInstance', () => {
365400
target_name: client.getClientType(),
366401
data: {},
367402
metadata: {},
403+
buffers: [],
368404
} as ToWebviewMessage]);
369405

370406
// Close the client.

src/vs/workbench/services/languageRuntime/common/positronIPyWidgetsWebviewMessages.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
/*---------------------------------------------------------------------------------------------
2-
* Copyright (C) 2024 Posit Software, PBC. All rights reserved.
2+
* Copyright (C) 2024-2025 Posit Software, PBC. All rights reserved.
33
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6+
import { VSBuffer } from '../../../../base/common/buffer.js';
7+
68
/**
79
* Request the runtime to close a comm.
810
*
@@ -99,6 +101,7 @@ export interface ICommOpenToWebview {
99101
comm_id: string;
100102
target_name: string;
101103
data: unknown;
104+
buffers?: Array<VSBuffer>;
102105
metadata: unknown;
103106
}
104107

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { IRuntimeClientEvent } from '../../../languageRuntime/common/languageRun
1313
import { TestRuntimeClientInstance } from '../../../languageRuntime/test/common/testRuntimeClientInstance.js';
1414
import { CancellationError } from '../../../../../base/common/errors.js';
1515
import { TestUiClientInstance } from '../../../languageRuntime/test/common/testUiClientInstance.js';
16+
import { VSBuffer } from '../../../../../base/common/buffer.js';
1617

1718
export class TestLanguageRuntimeSession extends Disposable implements ILanguageRuntimeSession {
1819
private readonly _onDidChangeRuntimeState = this._register(new Emitter<RuntimeState>());
@@ -140,7 +141,7 @@ export class TestLanguageRuntimeSession extends Disposable implements ILanguageR
140141
}
141142

142143
async createClient(
143-
type: RuntimeClientType, params: any, metadata?: any, id?: string
144+
type: RuntimeClientType, params: any, metadata?: any, id?: string, buffers?: VSBuffer[]
144145
): Promise<TestRuntimeClientInstance> {
145146
const client = type === RuntimeClientType.Ui ?
146147
new TestUiClientInstance(id ?? generateUuid()) :
@@ -163,7 +164,7 @@ export class TestLanguageRuntimeSession extends Disposable implements ILanguageR
163164
parent_id: '',
164165
type: LanguageRuntimeMessageType.CommOpen,
165166
when: new Date().toISOString(),
166-
buffers: [],
167+
buffers: buffers ?? [],
167168
}
168169
}
169170
);

0 commit comments

Comments
 (0)