From e10370bfd9346d046ed0ed15dbb67f09521bedc9 Mon Sep 17 00:00:00 2001 From: jankuca Date: Tue, 4 Nov 2025 14:27:36 +0100 Subject: [PATCH 1/3] add defensive handling of vscode api proposal usage --- .../controllers/vscodeNotebookController.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/notebooks/controllers/vscodeNotebookController.ts b/src/notebooks/controllers/vscodeNotebookController.ts index 66f2b7db4..49a97791a 100644 --- a/src/notebooks/controllers/vscodeNotebookController.ts +++ b/src/notebooks/controllers/vscodeNotebookController.ts @@ -173,10 +173,19 @@ export class VSCodeNotebookController implements Disposable, IVSCodeNotebookCont displayDataProvider ); - try { - controller.controller.variableProvider = jupyterVairablesProvider; - } catch (ex) { - logger.warn('Failed to attach variable provider', ex); + // Only attach variable provider if the API is available + // The notebookVariableProvider is a proposed API that: + // - Works in extension development mode (F5 debugging) + // - Does NOT work in published extensions from the Marketplace + // - Requires users to manually enable it with --enable-proposed-api flag + // See: https://code.visualstudio.com/api/advanced-topics/using-proposed-api + // This check allows the extension to gracefully degrade when the API is unavailable + if ('variableProvider' in controller.controller) { + try { + controller.controller.variableProvider = jupyterVairablesProvider; + } catch (ex) { + logger.warn('Failed to attach variable provider', ex); + } } return controller; From d7b31e83f28b78a051eeadbfa2c9e1c29d3dbc9a Mon Sep 17 00:00:00 2001 From: jankuca Date: Tue, 4 Nov 2025 14:38:53 +0100 Subject: [PATCH 2/3] add tests --- .../vscodeNotebookController.unit.test.ts | 190 +++++++++++++++++- 1 file changed, 189 insertions(+), 1 deletion(-) diff --git a/src/notebooks/controllers/vscodeNotebookController.unit.test.ts b/src/notebooks/controllers/vscodeNotebookController.unit.test.ts index cd835e0e7..f32972d96 100644 --- a/src/notebooks/controllers/vscodeNotebookController.unit.test.ts +++ b/src/notebooks/controllers/vscodeNotebookController.unit.test.ts @@ -37,11 +37,12 @@ import { KernelConnector } from './kernelConnector'; import { ITrustedKernelPaths } from '../../kernels/raw/finder/types'; import { IInterpreterService } from '../../platform/interpreter/contracts'; import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; -import { IConnectionDisplayDataProvider } from './types'; +import { IConnectionDisplayData, IConnectionDisplayDataProvider } from './types'; import { ConnectionDisplayDataProvider } from './connectionDisplayData.node'; import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock'; import { Environment, PythonExtension } from '@vscode/python-extension'; import { crateMockedPythonApi, whenResolveEnvironment } from '../../kernels/helpers.unit.test'; +import { IJupyterVariablesProvider } from '../../kernels/variables/types'; suite(`Notebook Controller`, function () { let controller: NotebookController; @@ -544,4 +545,191 @@ suite(`Notebook Controller`, function () { }); }); }); + + suite('VSCodeNotebookController.create', function () { + let kernelConnection: KernelConnectionMetadata; + let kernelProvider: IKernelProvider; + let context: IExtensionContext; + let languageService: NotebookCellLanguageService; + let configService: IConfigurationService; + let extensionChecker: IPythonExtensionChecker; + let serviceContainer: IServiceContainer; + let displayDataProvider: IConnectionDisplayDataProvider; + let jupyterVariablesProvider: IJupyterVariablesProvider; + let disposables: IDisposable[] = []; + let controller: NotebookController; + let onDidChangeSelectedNotebooks: EventEmitter<{ + readonly notebook: NotebookDocument; + readonly selected: boolean; + }>; + + setup(function () { + resetVSCodeMocks(); + disposables.push(new Disposable(() => resetVSCodeMocks())); + kernelConnection = mock(); + kernelProvider = mock(); + context = mock(); + languageService = mock(); + configService = mock(); + extensionChecker = mock(); + serviceContainer = mock(); + displayDataProvider = mock(); + jupyterVariablesProvider = mock(); + controller = mock(); + onDidChangeSelectedNotebooks = new EventEmitter<{ + readonly notebook: NotebookDocument; + readonly selected: boolean; + }>(); + disposables.push(onDidChangeSelectedNotebooks); + + when(context.extensionUri).thenReturn(Uri.file('extension')); + when(controller.onDidChangeSelectedNotebooks).thenReturn(onDidChangeSelectedNotebooks.event); + when(displayDataProvider.getDisplayData(anything())).thenReturn({ + label: 'Test Kernel', + description: 'Test Description', + detail: 'Test Detail', + category: 'Test Category', + serverDisplayName: 'Test Server', + onDidChange: new EventEmitter().event, + dispose: () => { + /* noop */ + } + }); + when( + mockedVSCodeNamespaces.notebooks.createNotebookController( + anything(), + anything(), + anything(), + anything(), + anything() + ) + ).thenReturn(instance(controller)); + }); + + teardown(() => (disposables = dispose(disposables))); + + test('Should attach variable provider when API is available', function () { + // Arrange: Mock controller with variableProvider property + const controllerWithApi = mock(); + when(controllerWithApi.onDidChangeSelectedNotebooks).thenReturn(onDidChangeSelectedNotebooks.event); + (instance(controllerWithApi) as any).variableProvider = undefined; + + when( + mockedVSCodeNamespaces.notebooks.createNotebookController( + anything(), + anything(), + anything(), + anything(), + anything() + ) + ).thenReturn(instance(controllerWithApi)); + + // Act + const result = VSCodeNotebookController.create( + instance(kernelConnection), + 'test-id', + 'jupyter-notebook', + instance(kernelProvider), + instance(context), + disposables, + instance(languageService), + instance(configService), + instance(extensionChecker), + instance(serviceContainer), + instance(displayDataProvider), + instance(jupyterVariablesProvider) + ); + + // Assert + assert.isDefined(result); + assert.strictEqual( + (result.controller as any).variableProvider, + instance(jupyterVariablesProvider), + 'Variable provider should be attached when API is available' + ); + }); + + test('Should not attach variable provider when API is not available', function () { + // Arrange: Mock controller without variableProvider property + const controllerWithoutApi = mock(); + when(controllerWithoutApi.onDidChangeSelectedNotebooks).thenReturn(onDidChangeSelectedNotebooks.event); + // Don't add variableProvider property to simulate API not being available + + when( + mockedVSCodeNamespaces.notebooks.createNotebookController( + anything(), + anything(), + anything(), + anything(), + anything() + ) + ).thenReturn(instance(controllerWithoutApi)); + + // Act + const result = VSCodeNotebookController.create( + instance(kernelConnection), + 'test-id', + 'jupyter-notebook', + instance(kernelProvider), + instance(context), + disposables, + instance(languageService), + instance(configService), + instance(extensionChecker), + instance(serviceContainer), + instance(displayDataProvider), + instance(jupyterVariablesProvider) + ); + + // Assert + assert.isDefined(result); + assert.isUndefined( + (result.controller as any).variableProvider, + 'Variable provider should not be attached when API is not available' + ); + }); + + test('Should handle errors when attaching variable provider', function () { + // Arrange: Mock controller that throws when setting variableProvider + const controllerWithError = mock(); + when(controllerWithError.onDidChangeSelectedNotebooks).thenReturn(onDidChangeSelectedNotebooks.event); + + const controllerInstance = instance(controllerWithError); + Object.defineProperty(controllerInstance, 'variableProvider', { + set: () => { + throw new Error('API not supported'); + }, + configurable: true + }); + + when( + mockedVSCodeNamespaces.notebooks.createNotebookController( + anything(), + anything(), + anything(), + anything(), + anything() + ) + ).thenReturn(controllerInstance); + + // Act - should not throw + const result = VSCodeNotebookController.create( + instance(kernelConnection), + 'test-id', + 'jupyter-notebook', + instance(kernelProvider), + instance(context), + disposables, + instance(languageService), + instance(configService), + instance(extensionChecker), + instance(serviceContainer), + instance(displayDataProvider), + instance(jupyterVariablesProvider) + ); + + // Assert + assert.isDefined(result); + }); + }); }); From 1b35f731e9bab0f5e766a66d64f165556188dc91 Mon Sep 17 00:00:00 2001 From: jankuca Date: Tue, 4 Nov 2025 14:57:25 +0100 Subject: [PATCH 3/3] fix test --- .../vscodeNotebookController.unit.test.ts | 40 +++++++++++++++---- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/src/notebooks/controllers/vscodeNotebookController.unit.test.ts b/src/notebooks/controllers/vscodeNotebookController.unit.test.ts index f32972d96..779aad570 100644 --- a/src/notebooks/controllers/vscodeNotebookController.unit.test.ts +++ b/src/notebooks/controllers/vscodeNotebookController.unit.test.ts @@ -650,10 +650,34 @@ suite(`Notebook Controller`, function () { }); test('Should not attach variable provider when API is not available', function () { - // Arrange: Mock controller without variableProvider property - const controllerWithoutApi = mock(); - when(controllerWithoutApi.onDidChangeSelectedNotebooks).thenReturn(onDidChangeSelectedNotebooks.event); - // Don't add variableProvider property to simulate API not being available + // Arrange: Create a plain object without variableProvider property + const controllerWithoutApi = { + onDidChangeSelectedNotebooks: onDidChangeSelectedNotebooks.event, + id: 'test-id', + notebookType: 'jupyter-notebook', + supportedLanguages: [], + supportsExecutionOrder: true, + description: '', + detail: '', + label: 'Test Kernel', + dispose: () => { + /* noop */ + }, + createNotebookCellExecution: () => ({}) as any, + createNotebookExecution: () => ({}) as any, + executeHandler: () => { + /* noop */ + }, + interruptHandler: undefined, + updateNotebookAffinity: () => { + /* noop */ + }, + rendererScripts: [], + onDidReceiveMessage: new EventEmitter().event, + postMessage: () => Promise.resolve(true), + asWebviewUri: (uri: Uri) => uri + // Note: no variableProvider property to simulate API not being available + } as NotebookController; when( mockedVSCodeNamespaces.notebooks.createNotebookController( @@ -663,7 +687,7 @@ suite(`Notebook Controller`, function () { anything(), anything() ) - ).thenReturn(instance(controllerWithoutApi)); + ).thenReturn(controllerWithoutApi); // Act const result = VSCodeNotebookController.create( @@ -683,9 +707,9 @@ suite(`Notebook Controller`, function () { // Assert assert.isDefined(result); - assert.isUndefined( - (result.controller as any).variableProvider, - 'Variable provider should not be attached when API is not available' + assert.isFalse( + 'variableProvider' in result.controller, + 'Variable provider property should not exist when API is not available' ); });