From eeaadd381baa27f49c1433e834e26c547bfe161c Mon Sep 17 00:00:00 2001 From: tomas Date: Wed, 1 Apr 2026 21:21:43 +0000 Subject: [PATCH 1/7] feat(toolkit): Simplify deepnote toolkit kernel management, to not use deepnote environments --- .../deepnote/deepnoteServerStarter.node.ts | 107 +-- .../deepnoteServerStarter.unit.test.ts | 16 +- src/kernels/deepnote/types.ts | 16 +- .../deepnoteKernelAutoSelector.node.ts | 639 ++---------------- ...epnoteKernelAutoSelector.node.unit.test.ts | 343 +++------- .../installer/pipInstaller.node.ts | 10 +- .../installer/productInstaller.node.ts | 17 +- .../interpreter/installer/productNames.ts | 1 + .../installer/productService.node.ts | 1 + src/platform/interpreter/installer/utils.ts | 2 + 10 files changed, 279 insertions(+), 873 deletions(-) diff --git a/src/kernels/deepnote/deepnoteServerStarter.node.ts b/src/kernels/deepnote/deepnoteServerStarter.node.ts index 112f7f0e09..ff2c7e2696 100644 --- a/src/kernels/deepnote/deepnoteServerStarter.node.ts +++ b/src/kernels/deepnote/deepnoteServerStarter.node.ts @@ -24,8 +24,10 @@ import { logger } from '../../platform/logging'; import { ISqlIntegrationEnvVarsProvider } from '../../platform/notebooks/deepnote/types'; import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; import * as path from '../../platform/vscode-path/path'; -import { DeepnoteServerInfo, IDeepnoteServerStarter, IDeepnoteToolkitInstaller } from './types'; +import { DeepnoteServerInfo, IDeepnoteServerStarter } from './types'; import { DeepnoteAgentSkillsManager } from './deepnoteAgentSkillsManager.node'; +import { getCachedEnvironment } from '../../platform/interpreter/helpers'; +import { IInstaller, InstallerResponse, Product } from '../../platform/interpreter/installer/types'; const MAX_OUTPUT_TRACKING_LENGTH = 5000; const SERVER_STARTUP_TIMEOUT_MS = 120_000; @@ -48,7 +50,7 @@ type PendingOperation = }; interface ProjectContext { - environmentId: string; + interpreterId: string; serverInfo: DeepnoteServerInfo | null; } @@ -71,7 +73,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension constructor( @inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory, - @inject(IDeepnoteToolkitInstaller) private readonly toolkitInstaller: IDeepnoteToolkitInstaller, + @inject(IInstaller) private readonly installer: IInstaller, @inject(DeepnoteAgentSkillsManager) private readonly agentSkillsManager: DeepnoteAgentSkillsManager, @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly outputChannel: IOutputChannel, @inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry, @@ -98,14 +100,11 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension */ public async startServer( interpreter: PythonEnvironment, - venvPath: Uri, - managedVenv: boolean, - additionalPackages: string[], - environmentId: string, deepnoteFileUri: Uri, token?: CancellationToken ): Promise { const fileKey = deepnoteFileUri.fsPath; + const interpreterId = interpreter.id; let pendingOp = this.pendingOperations.get(fileKey); if (pendingOp) { @@ -119,12 +118,12 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension let existingContext = this.projectContexts.get(fileKey); if (existingContext != null) { - const { environmentId: existingEnvironmentId, serverInfo: existingServerInfo } = existingContext; + const { interpreterId: existingInterpreterId, serverInfo: existingServerInfo } = existingContext; - if (existingEnvironmentId === environmentId) { + if (existingInterpreterId === interpreterId) { if (existingServerInfo != null && (await this.isServerRunning(existingServerInfo))) { logger.info( - `Deepnote server already running at ${existingServerInfo.url} for ${fileKey} (environmentId ${environmentId})` + `Deepnote server already running at ${existingServerInfo.url} for ${fileKey} (interpreter ${interpreterId})` ); return existingServerInfo; } @@ -136,14 +135,14 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension } } else { logger.info( - `Stopping existing server for ${fileKey} with environmentId ${existingEnvironmentId} to start new one with environmentId ${environmentId}...` + `Stopping existing server for ${fileKey} with interpreter ${existingInterpreterId} to start new one with interpreter ${interpreterId}...` ); await this.stopServerForEnvironment(existingContext, deepnoteFileUri, token); - existingContext.environmentId = environmentId; + existingContext.interpreterId = interpreterId; } } else { const newContext: ProjectContext = { - environmentId, + interpreterId, serverInfo: null }; @@ -153,16 +152,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension const operation = { type: 'start' as const, - promise: this.startServerForEnvironment( - existingContext, - interpreter, - venvPath, - managedVenv, - additionalPackages, - environmentId, - deepnoteFileUri, - token - ) + promise: this.startServerForEnvironment(existingContext, interpreter, deepnoteFileUri, token) }; this.pendingOperations.set(fileKey, operation); @@ -223,7 +213,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension * Core server start using @deepnote/runtime-core's `startServer`. * * Extension-specific layers: - * - Toolkit/venv installation (before start) + * - Toolkit check/install via IInstaller (before start) * - SQL integration env var injection (via ServerOptions.env) * - Lock file creation (after start, using returned PID) * - Output channel logging (via process stdout/stderr streams) @@ -231,37 +221,49 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension private async startServerForEnvironment( projectContext: ProjectContext, interpreter: PythonEnvironment, - venvPath: Uri, - managedVenv: boolean, - additionalPackages: string[], - environmentId: string, deepnoteFileUri: Uri, token?: CancellationToken ): Promise { const fileKey = deepnoteFileUri.fsPath; + const interpreterId = interpreter.id; Cancellation.throwIfCanceled(token); - logger.info(`Ensuring deepnote-toolkit is installed in venv for environment ${environmentId}...`); - const { pythonInterpreter: venvInterpreter } = await this.toolkitInstaller.ensureVenvAndToolkit( - interpreter, - venvPath, - managedVenv, - token - ); + // Check if deepnote-toolkit is installed, and install if needed + logger.info(`Checking deepnote-toolkit installation for interpreter ${interpreterId}...`); + const isInstalled = await this.installer.isInstalled(Product.deepnoteToolkit, interpreter); - this.agentSkillsManager.ensureSkillsUpdated(environmentId, venvInterpreter); + if (!isInstalled) { + logger.info(`deepnote-toolkit not installed, installing via IInstaller...`); + const { CancellationTokenSource } = await import('vscode'); + const cts = new CancellationTokenSource(); - Cancellation.throwIfCanceled(token); + try { + if (token) { + token.onCancellationRequested(() => cts.cancel()); + } + + const result = await this.installer.install(Product.deepnoteToolkit, interpreter, cts); + + if (result !== InstallerResponse.Installed) { + throw new Error('deepnote-toolkit installation was cancelled or failed'); + } + } finally { + cts.dispose(); + } + } - await this.toolkitInstaller.installAdditionalPackages(venvPath, additionalPackages, token); + this.agentSkillsManager.ensureSkillsUpdated(interpreterId, interpreter); Cancellation.throwIfCanceled(token); - logger.info(`Starting deepnote-toolkit server for ${fileKey} (environmentId ${environmentId})`); + // Derive the environment path from the interpreter + const envPath = this.deriveEnvPath(interpreter); + + logger.info(`Starting deepnote-toolkit server for ${fileKey} (interpreter ${interpreterId})`); this.outputChannel.appendLine(l10n.t('Starting Deepnote server...')); - const extraEnv = await this.gatherSqlIntegrationEnvVars(deepnoteFileUri, environmentId, token); + const extraEnv = await this.gatherSqlIntegrationEnvVars(deepnoteFileUri, interpreterId, token); // Initialize output tracking for error reporting this.serverOutputByFile.set(fileKey, { stdout: '', stderr: '' }); @@ -269,7 +271,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension let serverInfo: DeepnoteServerInfo | undefined; try { serverInfo = await startServer({ - pythonEnv: venvPath.fsPath, + pythonEnv: envPath, workingDirectory: path.dirname(deepnoteFileUri.fsPath), startupTimeoutMs: SERVER_STARTUP_TIMEOUT_MS, env: extraEnv @@ -307,6 +309,29 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension return serverInfo; } + /** + * Derive the environment path from a Python interpreter. + * Uses the cached environment info, or falls back to navigating up from the executable. + */ + private deriveEnvPath(interpreter: PythonEnvironment): string { + const cachedEnv = getCachedEnvironment(interpreter); + // eslint-disable-next-line local-rules/dont-use-fspath + const folderPath = cachedEnv?.environment?.folderUri?.fsPath; + + if (folderPath) { + return folderPath; + } + + const sysPrefix = cachedEnv?.executable?.sysPrefix; + + if (sysPrefix) { + return sysPrefix; + } + + // Fallback: go up from bin/python (or Scripts/python.exe on Windows) + return path.dirname(path.dirname(interpreter.uri.fsPath)); + } + /** * Stop the server using @deepnote/runtime-core's `stopServer` (SIGTERM -> wait -> SIGKILL). */ diff --git a/src/kernels/deepnote/deepnoteServerStarter.unit.test.ts b/src/kernels/deepnote/deepnoteServerStarter.unit.test.ts index f90e3a8ec1..2b7b37c2df 100644 --- a/src/kernels/deepnote/deepnoteServerStarter.unit.test.ts +++ b/src/kernels/deepnote/deepnoteServerStarter.unit.test.ts @@ -1,12 +1,13 @@ import { assert } from 'chai'; import * as fakeTimers from '@sinonjs/fake-timers'; import { anything, instance, mock, when } from 'ts-mockito'; +import { EventEmitter } from 'vscode'; import { DeepnoteAgentSkillsManager } from './deepnoteAgentSkillsManager.node'; import { DeepnoteServerStarter } from './deepnoteServerStarter.node'; import { IProcessServiceFactory } from '../../platform/common/process/types.node'; import { IAsyncDisposableRegistry, IOutputChannel } from '../../platform/common/types'; -import { IDeepnoteToolkitInstaller } from './types'; +import { IInstaller, InstallerResponse } from '../../platform/interpreter/installer/types'; import { ISqlIntegrationEnvVarsProvider } from '../../platform/notebooks/deepnote/types'; /** @@ -19,7 +20,7 @@ import { ISqlIntegrationEnvVarsProvider } from '../../platform/notebooks/deepnot suite('DeepnoteServerStarter', () => { let serverStarter: DeepnoteServerStarter; let mockProcessServiceFactory: IProcessServiceFactory; - let mockToolkitInstaller: IDeepnoteToolkitInstaller; + let mockInstaller: IInstaller; let mockAgentSkillsManager: DeepnoteAgentSkillsManager; let mockOutputChannel: IOutputChannel; let mockAsyncRegistry: IAsyncDisposableRegistry; @@ -32,7 +33,7 @@ suite('DeepnoteServerStarter', () => { setup(() => { mockProcessServiceFactory = mock(); - mockToolkitInstaller = mock(); + mockInstaller = mock(); mockAgentSkillsManager = mock(); mockOutputChannel = mock(); mockAsyncRegistry = mock(); @@ -40,10 +41,13 @@ suite('DeepnoteServerStarter', () => { when(mockAsyncRegistry.push(anything())).thenReturn(); when(mockOutputChannel.appendLine(anything())).thenReturn(); + when(mockInstaller.isInstalled(anything(), anything())).thenResolve(true); + when(mockInstaller.install(anything(), anything(), anything())).thenResolve(InstallerResponse.Installed); + when(mockInstaller.onInstalled).thenReturn(new EventEmitter().event); serverStarter = new DeepnoteServerStarter( instance(mockProcessServiceFactory), - instance(mockToolkitInstaller), + instance(mockInstaller), instance(mockAgentSkillsManager), instance(mockOutputChannel), instance(mockAsyncRegistry), @@ -62,7 +66,7 @@ suite('DeepnoteServerStarter', () => { // Create a starter without SQL provider const starterWithoutSql = new DeepnoteServerStarter( instance(mockProcessServiceFactory), - instance(mockToolkitInstaller), + instance(mockInstaller), instance(mockAgentSkillsManager), instance(mockOutputChannel), instance(mockAsyncRegistry) @@ -85,7 +89,7 @@ suite('DeepnoteServerStarter', () => { const starterWithCancelledSql = new DeepnoteServerStarter( instance(mockProcessServiceFactory), - instance(mockToolkitInstaller), + instance(mockInstaller), instance(mockAgentSkillsManager), instance(mockOutputChannel), instance(mockAsyncRegistry), diff --git a/src/kernels/deepnote/types.ts b/src/kernels/deepnote/types.ts index a0c17a31ba..444d3e6651 100644 --- a/src/kernels/deepnote/types.ts +++ b/src/kernels/deepnote/types.ts @@ -151,32 +151,24 @@ export interface IDeepnoteToolkitInstaller { export const IDeepnoteServerStarter = Symbol('IDeepnoteServerStarter'); export interface IDeepnoteServerStarter { /** - * Starts a deepnote-toolkit Jupyter server for a kernel environment. - * Environment-based method. + * Starts a deepnote-toolkit Jupyter server using the active Python interpreter. + * Handles checking/installing deepnote-toolkit via the IInstaller infrastructure. * @param interpreter The Python interpreter to use - * @param venvPath The path to the venv - * @param managedVenv Whether the venv is managed by this extension (created by us) - * @param environmentId The environment ID (for server management) * @param deepnoteFileUri The URI of the .deepnote file * @param token Cancellation token to cancel the operation * @returns Connection information (URL, port, etc.) */ startServer( interpreter: PythonEnvironment, - venvPath: vscode.Uri, - managedVenv: boolean, - additionalPackages: string[], - environmentId: string, deepnoteFileUri: vscode.Uri, token?: vscode.CancellationToken ): Promise; /** - * Stops the deepnote-toolkit server for a kernel environment. - * @param environmentId The environment ID + * Stops the deepnote-toolkit server for a .deepnote file. + * @param deepnoteFileUri The URI of the .deepnote file * @param token Cancellation token to cancel the operation */ - // stopServer(environmentId: string, token?: vscode.CancellationToken): Promise; stopServer(deepnoteFileUri: vscode.Uri, token?: vscode.CancellationToken): Promise; /** diff --git a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts index 63e7129200..e7988635ee 100644 --- a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts +++ b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts @@ -8,32 +8,23 @@ import { CancellationToken, CancellationTokenSource, Disposable, - NotebookController, NotebookControllerAffinity, NotebookDocument, - NotebookEditor, ProgressLocation, - QuickPickItem, Uri, commands, env, l10n, - notebooks, window, workspace } from 'vscode'; -import { DeepnoteEnvironment } from '../../kernels/deepnote/environments/deepnoteEnvironment'; import { DEEPNOTE_NOTEBOOK_TYPE, - DEEPNOTE_TOOLKIT_VERSION, DeepnoteKernelConnectionMetadata, - IDeepnoteEnvironmentManager, IDeepnoteKernelAutoSelector, IDeepnoteLspClientManager, - IDeepnoteNotebookEnvironmentMapper, IDeepnoteServerProvider, - IDeepnoteServerStarter, - IDeepnoteToolkitInstaller + IDeepnoteServerStarter } from '../../kernels/deepnote/types'; import { createJupyterConnectionInfo } from '../../kernels/jupyter/jupyterUtils'; import { JupyterLabHelper } from '../../kernels/jupyter/session/jupyterLabHelper'; @@ -51,7 +42,8 @@ import { getDisplayPath } from '../../platform/common/platform/fs-paths.node'; import { IConfigurationService, IDisposableRegistry, IOutputChannel } from '../../platform/common/types'; import { disposeAsync } from '../../platform/common/utils'; import { createDeepnoteServerConfigHandle } from '../../platform/deepnote/deepnoteServerUtils.node'; -import { DeepnoteKernelError, DeepnoteToolkitMissingError } from '../../platform/errors/deepnoteKernelErrors'; +import { DeepnoteKernelError } from '../../platform/errors/deepnoteKernelErrors'; +import { IInterpreterService } from '../../platform/interpreter/contracts'; import { logger } from '../../platform/logging'; import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; import { IControllerRegistration, IVSCodeNotebookController } from '../controllers/types'; @@ -60,10 +52,6 @@ import { IDeepnoteInitNotebookRunner } from './deepnoteInitNotebookRunner.node'; import { computeRequirementsHash } from './deepnoteProjectUtils'; import { IDeepnoteRequirementsHelper } from './deepnoteRequirementsHelper.node'; -// Constants for NotebookEditor retry logic -const NOTEBOOK_EDITOR_RETRY_COUNT = 10; -const NOTEBOOK_EDITOR_RETRY_DELAY_MS = 100; - /** * Automatically selects and starts Deepnote kernel for .deepnote notebooks */ @@ -73,10 +61,8 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, private readonly notebookConnectionMetadata = new Map(); // Track registered controllers per NOTEBOOK (full URI with query) - one controller per notebook private readonly notebookControllers = new Map(); - // Track environment for each notebook - private readonly notebookEnvironmentsIds = new Map(); - // Track per-notebook placeholder controllers for notebooks without configured environments - private readonly placeholderControllers = new Map(); + // Track interpreter ID for each notebook + private readonly notebookInterpreterIds = new Map(); // Track server handles per PROJECT (baseFileUri) - one server per project private readonly projectServerHandles = new Map(); // Track projects where we need to run init notebook (set during controller setup) @@ -100,12 +86,9 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, @inject(IDeepnoteNotebookManager) private readonly notebookManager: IDeepnoteNotebookManager, @inject(IKernelProvider) private readonly kernelProvider: IKernelProvider, @inject(IDeepnoteRequirementsHelper) private readonly requirementsHelper: IDeepnoteRequirementsHelper, - @inject(IDeepnoteEnvironmentManager) private readonly environmentManager: IDeepnoteEnvironmentManager, @inject(IDeepnoteServerStarter) private readonly serverStarter: IDeepnoteServerStarter, - @inject(IDeepnoteNotebookEnvironmentMapper) - private readonly notebookEnvironmentMapper: IDeepnoteNotebookEnvironmentMapper, @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly outputChannel: IOutputChannel, - @inject(IDeepnoteToolkitInstaller) private readonly toolkitInstaller: IDeepnoteToolkitInstaller + @inject(IInterpreterService) private readonly interpreterService: IInterpreterService ) {} public activate() { @@ -170,14 +153,11 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, (result) => { logger.info(`Auto-selecting Deepnote kernel for ${getDisplayPath(notebook.uri)} result: ${result}`); if (!result) { - logger.info(`No environment configured for ${getDisplayPath(notebook.uri)}, showing warning`); - this.showNoEnvironmentWarning(notebook).catch((error) => { - logger.error( - `Error showing no environment warning for ${getDisplayPath(notebook.uri)}`, - error - ); - void this.handleKernelSelectionError(error, notebook); - }); + logger.warn( + `No active Python interpreter found for ${getDisplayPath( + notebook.uri + )}, kernel not selected` + ); } }, (error) => { @@ -187,83 +167,6 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, ); } - private async showNoEnvironmentWarning(notebook: NotebookDocument): Promise { - logger.info(`Showing no environment warning for ${getDisplayPath(notebook.uri)}`); - const selectEnvironmentAction = l10n.t('Select Environment'); - const cancelAction = l10n.t('Cancel'); - - const selectedAction = await window.showWarningMessage( - l10n.t('No environment configured for this notebook. Please select an environment to continue.'), - { modal: false }, - selectEnvironmentAction, - cancelAction - ); - - logger.info(`Selected action: ${selectedAction}`); - if (selectedAction === selectEnvironmentAction) { - logger.info(`Executing command to pick environment for ${getDisplayPath(notebook.uri)}`); - void commands.executeCommand('deepnote.environments.selectForNotebook', { notebook }); - } - } - - public async pickEnvironment(notebookUri: Uri): Promise { - logger.info(`Picking environment for notebook ${getDisplayPath(notebookUri)}`); - - // Wait for environment manager to finish loading environments from storage - await this.environmentManager.waitForInitialization(); - - const environments = this.environmentManager.listEnvironments(); - const items: (QuickPickItem & { environment?: DeepnoteEnvironment })[] = environments.map((env) => { - return { - label: env.name, - description: getDisplayPath(env.pythonInterpreter.uri), - detail: env.packages?.length - ? l10n.t('Packages: {0}', env.packages.join(', ')) - : l10n.t('No additional packages'), - environment: env - }; - }); - - items.push({ - label: '$(add) Create New Environment', - description: 'Set up a new kernel environment', - alwaysShow: true - }); - - const selected = await window.showQuickPick(items, { - placeHolder: `Select an environment for ${getDisplayPath(notebookUri)}`, - matchOnDescription: true, - matchOnDetail: true - }); - - if (!selected) { - logger.info('User cancelled environment selection'); - return; // User cancelled - } - - if (!selected.environment) { - logger.info('User chose to create new environment - triggering create command'); - - await commands.executeCommand('deepnote.environments.create'); - - const newEnvironments = this.environmentManager.listEnvironments(); - - if (newEnvironments.length > environments.length) { - logger.info('Environment created, showing picker again'); - - return this.pickEnvironment(notebookUri); - } - - logger.info('No new environment created'); - - return; - } - - logger.info(`Selected environment "${selected.environment.name}" for notebook ${getDisplayPath(notebookUri)}`); - - return selected.environment; - } - private onControllerSelectionChanged(event: { notebook: NotebookDocument; controller: IVSCodeNotebookController; @@ -308,16 +211,6 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, } logger.info(`Deepnote notebook closed: ${getDisplayPath(notebook.uri)}`); - - // Clean up placeholder controller if it exists - const notebookKey = notebook.uri.toString(); - const placeholder = this.placeholderControllers.get(notebookKey); - - if (placeholder) { - logger.info(`Disposing placeholder controller for closed notebook: ${getDisplayPath(notebook.uri)}`); - placeholder.dispose(); - this.placeholderControllers.delete(notebookKey); - } } public async onKernelStarted(kernel: IKernel) { @@ -422,21 +315,17 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, // cause "command already exists" errors when trying to start new clients await this.lspClientManager.stopLspClients(notebook.uri, token); - // Update the controller with new environment's metadata - // Because we use notebook-based controller IDs, addOrUpdate will call updateConnection() - // on the existing controller instead of creating a new one - const environmentId = this.notebookEnvironmentMapper.getEnvironmentForNotebook(baseFileUri); - const environment = environmentId ? this.environmentManager.getEnvironment(environmentId) : undefined; + // Get the active interpreter and re-setup the kernel + const interpreter = await this.interpreterService.getActiveInterpreter(notebook.uri); - if (environment == null) { - await this.notebookEnvironmentMapper.removeEnvironmentForNotebook(baseFileUri); - logger.error(`No environment found for notebook ${getDisplayPath(notebook.uri)}`); + if (!interpreter) { + logger.error(`No active Python interpreter found for ${getDisplayPath(notebook.uri)}`); return; } - await this.ensureKernelSelectedWithConfiguration( + await this.ensureKernelSelectedWithInterpreter( notebook, - environment, + interpreter, baseFileUri, notebookKey, projectKey, @@ -459,27 +348,17 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, // projectKey identifies the PROJECT for server tracking const projectKey = baseFileUri.fsPath; - const environmentId = this.notebookEnvironmentMapper.getEnvironmentForNotebook(baseFileUri); - - if (environmentId == null) { - await this.selectPlaceholderController(notebook); + // Get the active Python interpreter + const interpreter = await this.interpreterService.getActiveInterpreter(notebook.uri); + if (!interpreter) { + logger.warn(`No active Python interpreter found for ${getDisplayPath(notebook.uri)}`); return false; } - const environment = environmentId ? this.environmentManager.getEnvironment(environmentId) : undefined; - - if (environment == null) { - logger.info(`No environment found for notebook ${getDisplayPath(notebook.uri)}`); - await this.notebookEnvironmentMapper.removeEnvironmentForNotebook(baseFileUri); - await this.selectPlaceholderController(notebook); - - return false; - } - - await this.ensureKernelSelectedWithConfiguration( + await this.ensureKernelSelectedWithInterpreter( notebook, - environment, + interpreter, baseFileUri, notebookKey, projectKey, @@ -490,26 +369,17 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, return true; } - public async ensureKernelSelectedWithConfiguration( + public async ensureKernelSelectedWithInterpreter( notebook: NotebookDocument, - configuration: DeepnoteEnvironment, + interpreter: PythonEnvironment, baseFileUri: Uri, notebookKey: string, projectKey: string, progress: { report(value: { message?: string; increment?: number }): void }, progressToken: CancellationToken ): Promise { - // Dispose placeholder controller if it exists (real controller is taking over) - const placeholder = this.placeholderControllers.get(notebookKey); - - if (placeholder) { - logger.info(`Disposing placeholder controller for ${getDisplayPath(notebook.uri)}`); - placeholder.dispose(); - this.placeholderControllers.delete(notebookKey); - } - - logger.info(`Setting up kernel using configuration: ${configuration.name} (${configuration.id})`); - progress.report({ message: `Using ${configuration.name}...` }); + logger.info(`Setting up kernel using interpreter: ${interpreter.id}`); + progress.report({ message: `Using interpreter ${getDisplayPath(interpreter.uri)}...` }); // Check if Python extension is installed if (!this.pythonExtensionChecker.isPythonExtensionInstalled) { @@ -519,71 +389,38 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, } const existingController = this.notebookControllers.get(notebookKey); - const existingEnvironmentId = this.notebookEnvironmentsIds.get(notebookKey); - - if (existingEnvironmentId != null && existingController != null && existingEnvironmentId === configuration.id) { - logger.info(`Existing controller found for notebook ${getDisplayPath(notebook.uri)}, verifying connection`); - - // Verify the controller's interpreter path matches the expected venv path - // This handles cases where notebooks were used in VS Code and now opened in Cursor - if (this.isControllerInterpreterValid(existingController, configuration.venvPath)) { - logger.info(`Controller verified, selecting it`); - await this.ensureControllerSelectedForNotebook(notebook, existingController, progressToken); - - return; - } - - const expectedInterpreter = this.getVenvInterpreterUri(configuration.venvPath); - logger.warn( - `Controller interpreter path mismatch! Expected: ${expectedInterpreter.fsPath}, Got: ${existingController.connection.interpreter?.uri.fsPath}. Recreating controller.` - ); + const existingInterpreterId = this.notebookInterpreterIds.get(notebookKey); - // Dispose old controller and recreate it - existingController.dispose(); - this.notebookControllers.delete(notebookKey); + if (existingInterpreterId != null && existingController != null && existingInterpreterId === interpreter.id) { + logger.info(`Existing controller found for notebook ${getDisplayPath(notebook.uri)}, reusing`); + await this.ensureControllerSelectedForNotebook(notebook, existingController, progressToken); + return; } // Ensure server is running (startServer is idempotent - returns early if already running) - // Note: startServer() will create the venv if it doesn't exist - logger.info(`Ensuring server is running for configuration ${configuration.id}`); + // Server starter handles toolkit check/install via IInstaller internally + logger.info(`Ensuring server is running for interpreter ${interpreter.id}`); progress.report({ message: 'Starting Deepnote server...' }); - const serverInfo = await this.serverStarter.startServer( - configuration.pythonInterpreter, - configuration.venvPath, - configuration.managedVenv, - configuration.packages ?? [], - configuration.id, - baseFileUri, - progressToken - ); + const serverInfo = await this.serverStarter.startServer(interpreter, baseFileUri, progressToken); - this.notebookEnvironmentsIds.set(notebookKey, configuration.id); + this.notebookInterpreterIds.set(notebookKey, interpreter.id); logger.info(`Server running at ${serverInfo.url}`); - // Update last used timestamp - await this.environmentManager.updateLastUsed(configuration.id); - - // Create server provider handle + // Create server provider handle using interpreter ID const serverProviderHandle: JupyterServerProviderHandle = { extensionId: JVSC_EXTENSION_ID, id: 'deepnote-server', - handle: createDeepnoteServerConfigHandle(configuration.id, baseFileUri) + handle: createDeepnoteServerConfigHandle(interpreter.id, baseFileUri) }; // Register the server with the provider (one server per PROJECT) this.serverProvider.registerServer(serverProviderHandle.handle, serverInfo); this.projectServerHandles.set(projectKey, serverProviderHandle.handle); - const lspInterpreterUri = this.getVenvInterpreterUri(configuration.venvPath); - - const lspInterpreter: PythonEnvironment = { - uri: lspInterpreterUri, - id: lspInterpreterUri.fsPath - } as PythonEnvironment; - + // Use the active interpreter directly for LSP (it already has deepnote-toolkit installed) try { - await this.lspClientManager.startLspClients(serverInfo, notebook.uri, lspInterpreter, progressToken); + await this.lspClientManager.startLspClients(serverInfo, notebook.uri, interpreter, progressToken); logger.info(`✓ LSP clients started for ${notebookKey}`); } catch (error) { @@ -592,12 +429,14 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, progress.report({ message: 'Connecting to kernel...' }); + const displayName = `Deepnote: ${getDisplayPath(interpreter.uri)} (${notebookKey})`; + const connectionInfo = createJupyterConnectionInfo( serverProviderHandle, { baseUrl: serverInfo.url, token: serverInfo.token || '', - displayName: `Deepnote: ${configuration.name} (${notebookKey})`, + displayName, authorizationHeader: {} }, this.requestCreator, @@ -612,8 +451,8 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, const kernelSpecs = await sessionManager.getKernelSpecs(); logger.info(`Available kernel specs on Deepnote server: ${kernelSpecs.map((s) => s.name).join(', ')}`); - // Use the extracted kernel selection logic - kernelSpec = this.selectKernelSpec(kernelSpecs, configuration.id); + // Select the default Python kernel (ipykernel-provided python3 spec) + kernelSpec = this.selectKernelSpec(kernelSpecs); logger.info(`✓ Using kernel spec: ${kernelSpec.name} (${kernelSpec.display_name})`); } finally { @@ -622,10 +461,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, progress.report({ message: 'Finalizing kernel setup...' }); - const venvInterpreter = this.getVenvInterpreterUri(configuration.venvPath); - - logger.info(`Using venv path: ${configuration.venvPath.fsPath}`); - logger.info(`Venv interpreter path: ${venvInterpreter.fsPath}`); + logger.info(`Using interpreter: ${interpreter.uri.fsPath}`); // CRITICAL: Use unique notebook-based ID (includes query with notebook ID) // This ensures each notebook gets its own controller/kernel, even within the same project. @@ -637,14 +473,14 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, const projectTitle = notebook.metadata?.deepnoteProjectName || 'Untitled Project'; const newConnectionMetadata = DeepnoteKernelConnectionMetadata.create({ - interpreter: { uri: venvInterpreter, id: venvInterpreter.fsPath }, + interpreter, kernelSpec, baseUrl: serverInfo.url, id: controllerId, projectFilePath: baseFileUri.toString(), serverProviderHandle, serverInfo, - environmentName: configuration.name, + environmentName: getDisplayPath(interpreter.uri), projectName: projectTitle, notebookName: notebookKey }); @@ -717,7 +553,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, // Auto-select the controller await this.ensureControllerSelectedForNotebook(notebook, controller, progressToken); - logger.info(`Successfully set up kernel with configuration: ${configuration.name}`); + logger.info(`Successfully set up kernel with interpreter: ${interpreter.id}`); progress.report({ message: 'Kernel ready!' }); } @@ -746,35 +582,20 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, } /** - * Select the appropriate kernel spec for an environment. + * Select the default Python kernel spec from the server. * Extracted for testability. * @param kernelSpecs Available kernel specs from the server - * @param environmentId The environment ID to find a kernel for * @returns The selected kernel spec * @throws Error if no suitable kernel spec is found */ - public selectKernelSpec(kernelSpecs: IJupyterKernelSpec[], environmentId: string): IJupyterKernelSpec { - // Look for environment-specific kernel first - const expectedKernelName = `deepnote-${environmentId}`; - logger.info(`Looking for environment-specific kernel: ${expectedKernelName}`); - - const kernelSpec = kernelSpecs.find((s) => s.name === expectedKernelName); + public selectKernelSpec(kernelSpecs: IJupyterKernelSpec[]): IJupyterKernelSpec { + const kernelSpec = + kernelSpecs.find((s) => s.language === 'python') || + kernelSpecs.find((s) => s.name === 'python3') || + kernelSpecs[0]; if (!kernelSpec) { - logger.warn( - `Environment-specific kernel '${expectedKernelName}' not found! Falling back to generic Python kernel.` - ); - // Fallback to any Python kernel - const fallbackKernel = - kernelSpecs.find((s) => s.language === 'python') || - kernelSpecs.find((s) => s.name === 'python3') || - kernelSpecs[0]; - - if (!fallbackKernel) { - throw new Error('No kernel specs available on Deepnote server'); - } - - return fallbackKernel; + throw new Error('No kernel specs available on Deepnote server'); } return kernelSpec; @@ -782,7 +603,7 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, /** * Ensure an environment is configured for the notebook before execution. - * If not configured, shows picker and sets up the kernel. + * Uses the active Python interpreter and the IInstaller infrastructure. * @returns true if environment is ready, false if user cancelled */ public async ensureEnvironmentConfiguredBeforeExecution( @@ -795,113 +616,20 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, const notebookKey = notebook.uri.toString(); const projectKey = baseFileUri.fsPath; - const existingEnvironmentId = this.notebookEnvironmentMapper.getEnvironmentForNotebook(baseFileUri); - - // No environment configured - need to pick one - if (!existingEnvironmentId) { - return this.pickAndSetupEnvironment(notebook, baseFileUri, notebookKey, projectKey, token); - } - - const environment = this.environmentManager.getEnvironment(existingEnvironmentId); + const interpreter = await this.interpreterService.getActiveInterpreter(notebook.uri); - // Environment no longer exists - remove stale mapping and pick a new one - if (!environment) { - logger.info(`Removing stale environment mapping for ${getDisplayPath(notebook.uri)}`); - await this.notebookEnvironmentMapper.removeEnvironmentForNotebook(baseFileUri); - - return this.pickAndSetupEnvironment(notebook, baseFileUri, notebookKey, projectKey, token); + if (!interpreter) { + logger.warn(`No active Python interpreter found for ${getDisplayPath(notebook.uri)}`); + return false; } const existingController = this.notebookControllers.get(notebookKey); - // Environment and controller already configured - but verify interpreter path still matches if (existingController) { - if (!this.isControllerInterpreterValid(existingController, environment.venvPath)) { - const expectedInterpreter = this.getVenvInterpreterUri(environment.venvPath); - logger.warn( - `Controller interpreter path mismatch! Expected: ${expectedInterpreter.fsPath}, Got: ${existingController.connection.interpreter?.uri.fsPath}. Recreating controller.` - ); - - existingController.dispose(); - this.notebookControllers.delete(notebookKey); - - return this.setupKernelForEnvironment( - notebook, - environment, - baseFileUri, - notebookKey, - projectKey, - token - ); - } - - logger.info(`Environment "${environment.name}" already configured for ${getDisplayPath(notebook.uri)}`); - + logger.info(`Controller already configured for ${getDisplayPath(notebook.uri)}`); return true; } - // Environment exists but controller is missing - set it up - logger.info( - `Environment "${environment.name}" configured but controller missing for ${getDisplayPath( - notebook.uri - )}, triggering setup` - ); - - return this.setupKernelForEnvironment(notebook, environment, baseFileUri, notebookKey, projectKey, token); - } - - /** - * Pick an environment and set up the kernel for a notebook. - */ - private async pickAndSetupEnvironment( - notebook: NotebookDocument, - baseFileUri: Uri, - notebookKey: string, - projectKey: string, - token: CancellationToken - ): Promise { - Cancellation.throwIfCanceled(token); - - logger.info(`No environment configured for ${getDisplayPath(notebook.uri)}, showing picker`); - const selectedEnvironment = await this.pickEnvironment(notebook.uri); - - if (!selectedEnvironment) { - logger.info(`User cancelled environment selection for ${getDisplayPath(notebook.uri)}`); - - return false; - } - - Cancellation.throwIfCanceled(token); - - await this.notebookEnvironmentMapper.setEnvironmentForNotebook(baseFileUri, selectedEnvironment.id); - - const result = await this.setupKernelForEnvironment( - notebook, - selectedEnvironment, - baseFileUri, - notebookKey, - projectKey, - token - ); - - if (result) { - logger.info(`Environment "${selectedEnvironment.name}" configured for ${getDisplayPath(notebook.uri)}`); - } - - return result; - } - - /** - * Set up the kernel for a given environment. - */ - private async setupKernelForEnvironment( - notebook: NotebookDocument, - environment: DeepnoteEnvironment, - baseFileUri: Uri, - notebookKey: string, - projectKey: string, - token: CancellationToken - ): Promise { try { await window.withProgress( { @@ -910,9 +638,9 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, cancellable: true }, async (progress, progressToken) => { - await this.ensureKernelSelectedWithConfiguration( + await this.ensureKernelSelectedWithInterpreter( notebook, - environment, + interpreter, baseFileUri, notebookKey, projectKey, @@ -924,23 +652,12 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, } catch (error) { if (token.isCancellationRequested || isCancellationError(error as Error)) { logger.info(`Kernel setup cancelled for ${getDisplayPath(notebook.uri)}`); - return false; } throw error; } - const createdController = this.notebookControllers.get(notebookKey); - - if (!createdController) { - logger.warn( - `Controller not created for "${environment.name}" on ${getDisplayPath(notebook.uri)} after setup` - ); - - return false; - } - - return true; + return !!this.notebookControllers.get(notebookKey); } /** @@ -964,106 +681,10 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, } } - private getVenvInterpreterUri(venvPath: Uri): Uri { - return process.platform === 'win32' - ? Uri.joinPath(venvPath, 'Scripts', 'python.exe') - : Uri.joinPath(venvPath, 'bin', 'python'); - } - - /** - * Check if a controller's interpreter path matches the expected venv path. - * Returns true when no interpreter is present (nothing to validate) or when paths match. - */ - private isControllerInterpreterValid( - controller: { connection: { interpreter?: { uri: Uri } } }, - venvPath: Uri - ): boolean { - const existingInterpreter = controller.connection.interpreter; - - if (!existingInterpreter) { - return true; - } - - const expectedInterpreter = this.getVenvInterpreterUri(venvPath); - - return existingInterpreter.uri.fsPath === expectedInterpreter.fsPath; - } - - /** - * Find the NotebookEditor for a given NotebookDocument. - * Required for properly selecting a kernel with the notebook.selectKernel command. - * Includes retry logic since the editor might not be visible immediately when the document opens. - */ - private async findNotebookEditor(notebook: NotebookDocument): Promise { - // Try to find immediately - let editor = window.visibleNotebookEditors.find((e) => e.notebook.uri.toString() === notebook.uri.toString()); - - if (editor) { - return editor; - } - - // If not found, wait briefly and retry (editor might not be visible yet) - for (let i = 0; i < NOTEBOOK_EDITOR_RETRY_COUNT; i++) { - await new Promise((resolve) => setTimeout(resolve, NOTEBOOK_EDITOR_RETRY_DELAY_MS)); - - editor = window.visibleNotebookEditors.find((e) => e.notebook.uri.toString() === notebook.uri.toString()); - - if (editor) { - return editor; - } - } - - return; - } - - /** - * Create and select a placeholder controller for a notebook without a configured environment. - */ - private async selectPlaceholderController(notebook: NotebookDocument): Promise { - const placeholder = this.createPlaceholderController(notebook); - placeholder.updateNotebookAffinity(notebook, NotebookControllerAffinity.Preferred); - - const notebookEditor = await this.findNotebookEditor(notebook); - - if (notebookEditor) { - await commands.executeCommand('notebook.selectKernel', { - notebookEditor: notebookEditor, - id: placeholder.id, - extension: JVSC_EXTENSION_ID - }); - } else { - logger.warn( - `Could not find NotebookEditor for ${getDisplayPath(notebook.uri)}, kernel may not be selected` - ); - } - } - /** * Handle kernel selection errors with user-friendly messages and actions */ - public async handleKernelSelectionError(error: unknown, notebook: NotebookDocument): Promise { - if (error instanceof DeepnoteToolkitMissingError) { - const installAction = l10n.t('Install'); - const changeEnvironmentAction = l10n.t('Change Environment'); - const selectedAction = await window.showWarningMessage( - l10n.t( - 'Running Deepnote projects requires deepnote-toolkit[server]=={0} to be installed in the selected environment', - DEEPNOTE_TOOLKIT_VERSION - ), - { modal: true }, - installAction, - changeEnvironmentAction - ); - - if (selectedAction === installAction) { - await this.installToolkitAndNotify(error.venvPath, notebook); - } else if (selectedAction === changeEnvironmentAction) { - void commands.executeCommand('deepnote.environments.selectForNotebook', { notebook }); - } - - return; - } - + public async handleKernelSelectionError(error: unknown, _notebook: NotebookDocument): Promise { // Handle DeepnoteKernelError types with specific guidance if (error instanceof DeepnoteKernelError) { // Log the technical details @@ -1117,35 +738,6 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, } } - /** - * Install deepnote-toolkit in an existing venv and rebuild the controller. - */ - private async installToolkitAndNotify(venvPath: string, notebook: NotebookDocument): Promise { - try { - await window.withProgress( - { - location: ProgressLocation.Notification, - title: l10n.t('Installing deepnote-toolkit...'), - cancellable: true - }, - async (progress, token) => { - await this.toolkitInstaller.installToolkitInExistingVenv(Uri.file(venvPath), token); - - // After successful installation, rebuild the controller to use the new environment - progress.report({ message: l10n.t('Starting kernel...') }); - await this.rebuildController(notebook, progress, token); - } - ); - - void window.showInformationMessage(l10n.t('deepnote-toolkit installed successfully')); - } catch (installError) { - logger.error('Failed to install deepnote-toolkit', installError); - const errorMessage = installError instanceof Error ? installError.message : String(installError); - - void window.showErrorMessage(l10n.t('Failed to install deepnote-toolkit: {0}', errorMessage)); - } - } - /** * Read and hash the existing requirements.txt file if it exists. * Returns the same hash format as computeRequirementsHash for comparison. @@ -1181,103 +773,4 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, return ''; } } - - /** - * Create a placeholder controller for a notebook without a configured environment. - * Each notebook gets its own placeholder with a unique ID. - * The placeholder's executeHandler shows the environment picker when user tries to run cells. - */ - private createPlaceholderController(notebook: NotebookDocument): NotebookController { - const notebookKey = notebook.uri.toString(); - - // Check if we already have one - const existing = this.placeholderControllers.get(notebookKey); - - if (existing) { - return existing; - } - - const controller = notebooks.createNotebookController( - `deepnote-placeholder-${notebookKey}`, - DEEPNOTE_NOTEBOOK_TYPE, - l10n.t('Deepnote: Select Environment') - ); - - controller.supportsExecutionOrder = true; - controller.supportedLanguages = ['python', 'sql', 'markdown']; - - // Execution handler that shows environment picker when user tries to run without an environment - controller.executeHandler = async (cells, doc) => { - logger.info( - `Placeholder controller execute handler called for ${getDisplayPath(doc.uri)} with ${ - cells.length - } cells` - ); - - // Create a cancellation token that cancels when the notebook is closed - const cts = new CancellationTokenSource(); - const closeListener = workspace.onDidCloseNotebookDocument((closedDoc) => { - if (closedDoc.uri.toString() === doc.uri.toString()) { - logger.info(`Notebook closed during environment setup, cancelling operation`); - cts.cancel(); - } - }); - - try { - const hasEnvironment = await this.ensureEnvironmentConfiguredBeforeExecution(doc, cts.token); - - if (!hasEnvironment) { - logger.info(`User cancelled environment selection, not executing cells`); - - return; - } - - // Environment is now configured, execute the cells through the kernel - const docNotebookKey = doc.uri.toString(); - const realController = this.notebookControllers.get(docNotebookKey); - - if (!realController) { - logger.error(`No controller found after environment configuration for ${docNotebookKey}`); - - return; - } - - logger.info(`Executing ${cells.length} cells through kernel after environment configuration`); - - // Get or create a kernel for this notebook with the new connection - const kernel = this.kernelProvider.getOrCreate(doc, { - metadata: realController.connection, - controller: realController.controller, - resourceUri: doc.uri - }); - - // Execute cells through the kernel - const kernelExecution = this.kernelProvider.getKernelExecution(kernel); - - for (const cell of cells) { - try { - await kernelExecution.executeCell(cell); - } catch (cellError) { - logger.error(`Error executing cell ${cell.index}`, cellError); - // Continue with remaining cells - } - } - - logger.info(`Finished executing ${cells.length} cells`); - } catch (error) { - if (isCancellationError(error)) { - logger.info(`Environment setup cancelled for ${getDisplayPath(doc.uri)}`); - } else { - logger.error(`Error in placeholder controller execute handler`, error); - } - } finally { - closeListener.dispose(); - cts.dispose(); - } - }; - - this.placeholderControllers.set(notebookKey, controller); - - return controller; - } } diff --git a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts index 824635343c..cb0b67ee69 100644 --- a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts @@ -2,14 +2,10 @@ import { assert } from 'chai'; import * as sinon from 'sinon'; import { anything, instance, mock, verify, when } from 'ts-mockito'; import { DeepnoteKernelAutoSelector } from './deepnoteKernelAutoSelector.node'; -import { createMockChildProcess } from '../../kernels/deepnote/deepnoteTestHelpers.node'; import { - IDeepnoteEnvironmentManager, IDeepnoteLspClientManager, - IDeepnoteNotebookEnvironmentMapper, IDeepnoteServerProvider, - IDeepnoteServerStarter, - IDeepnoteToolkitInstaller + IDeepnoteServerStarter } from '../../kernels/deepnote/types'; import { IControllerRegistration, IVSCodeNotebookController } from '../controllers/types'; import { IDisposableRegistry, IOutputChannel } from '../../platform/common/types'; @@ -21,8 +17,8 @@ import { IDeepnoteNotebookManager } from '../types'; import { IKernelProvider, IKernel, IJupyterKernelSpec } from '../../kernels/types'; import { IDeepnoteRequirementsHelper } from './deepnoteRequirementsHelper.node'; import { NotebookDocument, Uri, NotebookController, CancellationToken } from 'vscode'; -import { DeepnoteEnvironment } from '../../kernels/deepnote/environments/deepnoteEnvironment'; import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; +import { IInterpreterService } from '../../platform/interpreter/contracts'; import { computeRequirementsHash } from './deepnoteProjectUtils'; import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock'; @@ -39,11 +35,9 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { let mockNotebookManager: IDeepnoteNotebookManager; let mockKernelProvider: IKernelProvider; let mockRequirementsHelper: IDeepnoteRequirementsHelper; - let mockEnvironmentManager: IDeepnoteEnvironmentManager; let mockServerStarter: IDeepnoteServerStarter; - let mockNotebookEnvironmentMapper: IDeepnoteNotebookEnvironmentMapper; let mockOutputChannel: IOutputChannel; - let mockToolkitInstaller: IDeepnoteToolkitInstaller; + let mockInterpreterService: IInterpreterService; let mockProgress: { report(value: { message?: string; increment?: number }): void }; let mockCancellationToken: CancellationToken; @@ -69,11 +63,9 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { mockNotebookManager = mock(); mockKernelProvider = mock(); mockRequirementsHelper = mock(); - mockEnvironmentManager = mock(); mockServerStarter = mock(); - mockToolkitInstaller = mock(); - mockNotebookEnvironmentMapper = mock(); mockOutputChannel = mock(); + mockInterpreterService = mock(); mockProgress = { report: sandbox.stub() }; mockCancellationToken = mock(); @@ -135,11 +127,9 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { instance(mockNotebookManager), instance(mockKernelProvider), instance(mockRequirementsHelper), - instance(mockEnvironmentManager), instance(mockServerStarter), - instance(mockNotebookEnvironmentMapper), instance(mockOutputChannel), - instance(mockToolkitInstaller) + instance(mockInterpreterService) ); }); @@ -158,31 +148,31 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { pendingCells: [{ index: 0 }, { index: 1 }] // 2 cells pending }; - // Create mock environment - const mockEnvironment = createMockEnvironment('test-env-id', 'Test Environment'); - - // Mock environment mapper and manager - when(mockNotebookEnvironmentMapper.getEnvironmentForNotebook(anything())).thenReturn('test-env-id'); - when(mockEnvironmentManager.getEnvironment('test-env-id')).thenReturn(mockEnvironment); + // Mock interpreter service to return an active interpreter + const mockInterpreter: PythonEnvironment = { + id: '/usr/bin/python3', + uri: Uri.parse('/usr/bin/python3') + }; + when(mockInterpreterService.getActiveInterpreter(anything())).thenResolve(mockInterpreter); when(mockKernelProvider.get(mockNotebook)).thenReturn(instance(mockKernel)); when(mockKernelProvider.getKernelExecution(instance(mockKernel))).thenReturn(mockExecution as any); - // Stub ensureKernelSelectedWithConfiguration to verify it's still called despite pending cells - const ensureKernelSelectedWithConfigurationStub = sandbox - .stub(selector, 'ensureKernelSelectedWithConfiguration') + // Stub ensureKernelSelectedWithInterpreter to verify it's still called despite pending cells + const ensureKernelSelectedWithInterpreterStub = sandbox + .stub(selector, 'ensureKernelSelectedWithInterpreter') .resolves(); // Act await selector.rebuildController(mockNotebook, mockProgress, instance(mockCancellationToken)); // Assert - should proceed despite pending cells assert.strictEqual( - ensureKernelSelectedWithConfigurationStub.calledOnce, + ensureKernelSelectedWithInterpreterStub.calledOnce, true, 'ensureKernelSelected should be called even with pending cells' ); assert.strictEqual( - ensureKernelSelectedWithConfigurationStub.firstCall.args[0], + ensureKernelSelectedWithInterpreterStub.firstCall.args[0], mockNotebook, 'ensureKernelSelected should be called with the notebook' ); @@ -195,16 +185,16 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { // Arrange when(mockKernelProvider.get(mockNotebook)).thenReturn(undefined); - // Create mock environment - const mockEnvironment = createMockEnvironment('test-env-id', 'Test Environment'); - - // Mock environment mapper and manager - when(mockNotebookEnvironmentMapper.getEnvironmentForNotebook(anything())).thenReturn('test-env-id'); - when(mockEnvironmentManager.getEnvironment('test-env-id')).thenReturn(mockEnvironment); + // Mock interpreter service to return an active interpreter + const mockInterpreter: PythonEnvironment = { + id: '/usr/bin/python3', + uri: Uri.parse('/usr/bin/python3') + }; + when(mockInterpreterService.getActiveInterpreter(anything())).thenResolve(mockInterpreter); - // Stub ensureKernelSelectedWithConfiguration to verify it's called - const ensureKernelSelectedWithConfigurationStub = sandbox - .stub(selector, 'ensureKernelSelectedWithConfiguration') + // Stub ensureKernelSelectedWithInterpreter to verify it's called + const ensureKernelSelectedWithInterpreterStub = sandbox + .stub(selector, 'ensureKernelSelectedWithInterpreter') .resolves(); // Act @@ -212,34 +202,34 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { // Assert - should proceed normally without a kernel assert.strictEqual( - ensureKernelSelectedWithConfigurationStub.calledOnce, + ensureKernelSelectedWithInterpreterStub.calledOnce, true, 'ensureKernelSelected should be called even when no kernel exists' ); assert.strictEqual( - ensureKernelSelectedWithConfigurationStub.firstCall.args[0], + ensureKernelSelectedWithInterpreterStub.firstCall.args[0], mockNotebook, 'ensureKernelSelected should be called with the notebook' ); }); - test('should complete successfully and delegate to ensureKernelSelectedWithConfiguration', async () => { - // This test verifies that ensureKernelSelectedWithConfiguration completes successfully + test('should complete successfully and delegate to ensureKernelSelectedWithInterpreter', async () => { + // This test verifies that ensureKernelSelectedWithInterpreter completes successfully // and delegates kernel setup to ensureKernelSelected // Arrange when(mockKernelProvider.get(mockNotebook)).thenReturn(undefined); - // Create mock environment - const mockEnvironment = createMockEnvironment('test-env-id', 'Test Environment'); - - // Mock environment mapper and manager - when(mockNotebookEnvironmentMapper.getEnvironmentForNotebook(anything())).thenReturn('test-env-id'); - when(mockEnvironmentManager.getEnvironment('test-env-id')).thenReturn(mockEnvironment); + // Mock interpreter service to return an active interpreter + const mockInterpreter: PythonEnvironment = { + id: '/usr/bin/python3', + uri: Uri.parse('/usr/bin/python3') + }; + when(mockInterpreterService.getActiveInterpreter(anything())).thenResolve(mockInterpreter); - // Stub ensureKernelSelectedWithConfiguration to verify delegation - const ensureKernelSelectedWithConfigurationStub = sandbox - .stub(selector, 'ensureKernelSelectedWithConfiguration') + // Stub ensureKernelSelectedWithInterpreter to verify delegation + const ensureKernelSelectedWithInterpreterStub = sandbox + .stub(selector, 'ensureKernelSelectedWithInterpreter') .resolves(); // Act @@ -247,30 +237,30 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { // Assert - method should complete without errors assert.strictEqual( - ensureKernelSelectedWithConfigurationStub.calledOnce, + ensureKernelSelectedWithInterpreterStub.calledOnce, true, - 'ensureKernelSelectedWithConfiguration should be called to set up the new environment' + 'ensureKernelSelectedWithInterpreter should be called to set up the new environment' ); }); - test('should pass cancellation token to ensureKernelSelectedWithConfiguration', async () => { + test('should pass cancellation token to ensureKernelSelectedWithInterpreter', async () => { // This test verifies that rebuildController correctly passes the cancellation token - // to ensureKernelSelectedWithConfiguration, allowing the operation to be cancelled during execution + // to ensureKernelSelectedWithInterpreter, allowing the operation to be cancelled during execution // Arrange when(mockCancellationToken.isCancellationRequested).thenReturn(true); when(mockKernelProvider.get(mockNotebook)).thenReturn(undefined); - // Create mock environment - const mockEnvironment = createMockEnvironment('test-env-id', 'Test Environment'); - - // Mock environment mapper and manager - when(mockNotebookEnvironmentMapper.getEnvironmentForNotebook(anything())).thenReturn('test-env-id'); - when(mockEnvironmentManager.getEnvironment('test-env-id')).thenReturn(mockEnvironment); + // Mock interpreter service to return an active interpreter + const mockInterpreter: PythonEnvironment = { + id: '/usr/bin/python3', + uri: Uri.parse('/usr/bin/python3') + }; + when(mockInterpreterService.getActiveInterpreter(anything())).thenResolve(mockInterpreter); - // Stub ensureKernelSelectedWithConfiguration to verify it receives the token - const ensureKernelSelectedWithConfigurationStub = sandbox - .stub(selector, 'ensureKernelSelectedWithConfiguration') + // Stub ensureKernelSelectedWithInterpreter to verify it receives the token + const ensureKernelSelectedWithInterpreterStub = sandbox + .stub(selector, 'ensureKernelSelectedWithInterpreter') .resolves(); // Act @@ -278,50 +268,23 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { // Assert assert.strictEqual( - ensureKernelSelectedWithConfigurationStub.calledOnce, + ensureKernelSelectedWithInterpreterStub.calledOnce, true, - 'ensureKernelSelectedWithConfiguration should be called once' + 'ensureKernelSelectedWithInterpreter should be called once' ); assert.strictEqual( - ensureKernelSelectedWithConfigurationStub.firstCall.args[0], + ensureKernelSelectedWithInterpreterStub.firstCall.args[0], mockNotebook, 'ensureKernelSelected should be called with the notebook' ); assert.strictEqual( - ensureKernelSelectedWithConfigurationStub.firstCall.args[6], + ensureKernelSelectedWithInterpreterStub.firstCall.args[6], instance(mockCancellationToken), 'ensureKernelSelected should be called with the cancellation token' ); }); }); - suite('pickEnvironment', () => { - test('should return selected environment when user picks one', async () => { - // Arrange - const notebookUri = Uri.parse('file:///test/notebook.deepnote'); - const mockEnv1 = createMockEnvironment('env-1', 'Environment 1'); - const mockEnv2 = createMockEnvironment('env-2', 'Environment 2'); - const environments = [mockEnv1, mockEnv2]; - - // Mock environment manager - when(mockEnvironmentManager.waitForInitialization()).thenResolve(); - when(mockEnvironmentManager.listEnvironments()).thenReturn(environments); - - // Mock window.showQuickPick to simulate user selecting the first environment - when(mockedVSCodeNamespaces.window.showQuickPick(anything(), anything())).thenResolve({ - label: mockEnv1.name, - description: mockEnv1.pythonInterpreter.uri.fsPath, - environment: mockEnv1 - } as any); - - // Act - const result = await selector.pickEnvironment(notebookUri); - - // Assert - assert.strictEqual(result, mockEnv1, 'Should return the selected environment'); - }); - }); - suite('onKernelStarted', () => { test('should return early and not call initNotebookRunner for non-deepnote notebooks', async () => { // Arrange @@ -343,51 +306,12 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { }); suite('ensureKernelSelected', () => { - test('should return false when no environment ID is assigned to the notebook', async () => { - // Mock environment mapper to return null (no environment assigned) - when(mockNotebookEnvironmentMapper.getEnvironmentForNotebook(anything())).thenReturn(undefined); - - // Stub ensureKernelSelectedWithConfiguration to track if it gets called - const ensureKernelSelectedStub = sandbox.stub(selector, 'ensureKernelSelectedWithConfiguration').resolves(); - - // Mock commands.executeCommand - when(mockedVSCodeNamespaces.commands.executeCommand(anything(), anything())).thenResolve(); - - // Act - const result = await selector.ensureKernelSelected( - mockNotebook, - mockProgress, - instance(mockCancellationToken) - ); - - // Assert - assert.strictEqual(result, false, 'Should return false when no environment is assigned'); - assert.strictEqual( - ensureKernelSelectedStub.called, - false, - 'ensureKernelSelectedWithConfiguration should not be called' - ); - verify(mockNotebookEnvironmentMapper.getEnvironmentForNotebook(anything())).once(); - }); - - test('should return false and remove mapping when environment is not found', async () => { - // Arrange - const environmentId = 'missing-env-id'; - - // Mock environment mapper to return an ID - when(mockNotebookEnvironmentMapper.getEnvironmentForNotebook(anything())).thenReturn(environmentId); + test('should return false when no active interpreter is found', async () => { + // Mock interpreter service to return undefined (no active interpreter) + when(mockInterpreterService.getActiveInterpreter(anything())).thenResolve(undefined); - // Mock environment manager to return null (environment not found) - when(mockEnvironmentManager.getEnvironment(environmentId)).thenReturn(undefined); - - // Mock remove environment mapping - when(mockNotebookEnvironmentMapper.removeEnvironmentForNotebook(anything())).thenResolve(); - - // Stub ensureKernelSelectedWithConfiguration to track if it gets called - const ensureKernelSelectedStub = sandbox.stub(selector, 'ensureKernelSelectedWithConfiguration').resolves(); - - // Mock commands.executeCommand - when(mockedVSCodeNamespaces.commands.executeCommand(anything(), anything())).thenResolve(); + // Stub ensureKernelSelectedWithInterpreter to track if it gets called + const ensureKernelSelectedStub = sandbox.stub(selector, 'ensureKernelSelectedWithInterpreter').resolves(); // Act const result = await selector.ensureKernelSelected( @@ -397,36 +321,29 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { ); // Assert - assert.strictEqual(result, false, 'Should return false when environment is not found'); + assert.strictEqual(result, false, 'Should return false when no active interpreter is found'); assert.strictEqual( ensureKernelSelectedStub.called, false, - 'ensureKernelSelectedWithConfiguration should not be called' + 'ensureKernelSelectedWithInterpreter should not be called' ); - verify(mockNotebookEnvironmentMapper.getEnvironmentForNotebook(anything())).once(); - verify(mockEnvironmentManager.getEnvironment(environmentId)).once(); - verify(mockNotebookEnvironmentMapper.removeEnvironmentForNotebook(anything())).once(); }); - test('should return true and call ensureKernelSelectedWithConfiguration when environment is found', async () => { + test('should return true and call ensureKernelSelectedWithInterpreter when interpreter is found', async () => { // Arrange const baseFileUri = mockNotebook.uri.with({ query: '', fragment: '' }); const notebookKey = mockNotebook.uri.toString(); const projectKey = baseFileUri.fsPath; - const environmentId = 'test-env-id'; - const mockEnvironment = createMockEnvironment(environmentId, 'Test Environment'); - - // Mock environment mapper to return an ID - when(mockNotebookEnvironmentMapper.getEnvironmentForNotebook(anything())).thenReturn(environmentId); - - // Mock environment manager to return the environment - when(mockEnvironmentManager.getEnvironment(environmentId)).thenReturn(mockEnvironment); + const mockInterpreter: PythonEnvironment = { + id: '/usr/bin/python3', + uri: Uri.parse('/usr/bin/python3') + }; - // Stub ensureKernelSelectedWithConfiguration to track calls - const ensureKernelSelectedStub = sandbox.stub(selector, 'ensureKernelSelectedWithConfiguration').resolves(); + // Mock interpreter service to return an active interpreter + when(mockInterpreterService.getActiveInterpreter(anything())).thenResolve(mockInterpreter); - // Mock commands.executeCommand - when(mockedVSCodeNamespaces.commands.executeCommand(anything(), anything())).thenResolve(); + // Stub ensureKernelSelectedWithInterpreter to track calls + const ensureKernelSelectedStub = sandbox.stub(selector, 'ensureKernelSelectedWithInterpreter').resolves(); // Act const result = await selector.ensureKernelSelected( @@ -436,25 +353,22 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { ); // Assert - assert.strictEqual(result, true, 'Should return true when environment is found'); + assert.strictEqual(result, true, 'Should return true when interpreter is found'); assert.strictEqual( ensureKernelSelectedStub.calledOnce, true, - 'ensureKernelSelectedWithConfiguration should be called once' + 'ensureKernelSelectedWithInterpreter should be called once' ); // Verify it was called with correct arguments const callArgs = ensureKernelSelectedStub.firstCall.args; assert.strictEqual(callArgs[0], mockNotebook, 'First arg should be notebook'); - assert.strictEqual(callArgs[1], mockEnvironment, 'Second arg should be environment'); + assert.deepStrictEqual(callArgs[1], mockInterpreter, 'Second arg should be interpreter'); assert.strictEqual(callArgs[2].toString(), baseFileUri.toString(), 'Third arg should be baseFileUri'); assert.strictEqual(callArgs[3], notebookKey, 'Fourth arg should be notebookKey'); assert.strictEqual(callArgs[4], projectKey, 'Fifth arg should be projectKey'); assert.strictEqual(callArgs[5], mockProgress, 'Sixth arg should be progress'); assert.strictEqual(callArgs[6], instance(mockCancellationToken), 'Seventh arg should be token'); - - verify(mockNotebookEnvironmentMapper.getEnvironmentForNotebook(anything())).once(); - verify(mockEnvironmentManager.getEnvironment(environmentId)).once(); }); }); @@ -702,7 +616,7 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { // // CURRENT IMPLEMENTATION BEHAVIOR: // - // 1. If startServer() fails, the error propagates from ensureKernelSelectedWithConfiguration() + // 1. If startServer() fails, the error propagates from ensureKernelSelectedWithInterpreter() // (deepnoteKernelAutoSelector.node.ts:450-467) // // 2. The error is caught and shown to user in the UI layer @@ -738,72 +652,53 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { // REAL TDD Tests - These should FAIL if bugs exist suite('Bug Detection: Kernel Selection', () => { - test('BUG-1: Should prefer environment-specific kernel over .env kernel', () => { - // REAL TEST: This will FAIL if the wrong kernel is selected - // - // The selectKernelSpec method is now extracted and testable! + test('Should select the first Python kernel from available specs', () => { + // The selectKernelSpec method selects the first Python kernel available - const envId = 'env123'; const kernelSpecs: IJupyterKernelSpec[] = [ createMockKernelSpec('.env', '.env Python', 'python'), - createMockKernelSpec(`deepnote-${envId}`, 'Deepnote Environment', 'python'), createMockKernelSpec('python3', 'Python 3', 'python') ]; - const selected = selector.selectKernelSpec(kernelSpecs, envId); + const selected = selector.selectKernelSpec(kernelSpecs); - // CRITICAL ASSERTION: Should select environment-specific kernel, NOT .env - assert.strictEqual( - selected?.name, - `deepnote-${envId}`, - `BUG DETECTED: Selected "${selected?.name}" instead of "deepnote-${envId}"! This would use wrong environment.` - ); + // Should select the first Python kernel + assert.strictEqual(selected.language, 'python', 'Should select a Python kernel'); + assert.strictEqual(selected.name, '.env', 'Should select the first Python kernel'); }); - test('BUG-1b: Current implementation falls back to Python kernel (documents expected behavior)', () => { - // This test documents that the current implementation DOES have fallback logic - // - // EXPECTED BEHAVIOR (current): Fall back to generic Python kernel when env-specific kernel not found - // This is a design decision - we don't want to block users if the environment-specific kernel isn't ready yet + test('Should fall back to python3 named kernel when no python language kernel exists first', () => { + // Documents fallback behavior - finds python3 by name if no python language match - const envId = 'env123'; const kernelSpecs: IJupyterKernelSpec[] = [ - createMockKernelSpec('.env', '.env Python', 'python'), + createMockKernelSpec('javascript', 'JavaScript', 'javascript'), createMockKernelSpec('python3', 'Python 3', 'python') ]; - // Should fall back to a Python kernel (this is the current behavior) - const selected = selector.selectKernelSpec(kernelSpecs, envId); + const selected = selector.selectKernelSpec(kernelSpecs); - // Should have selected a fallback kernel (either .env or python3) - assert.ok(selected, 'Should select a fallback kernel'); - assert.strictEqual(selected.language, 'python', 'Fallback should be a Python kernel'); + assert.strictEqual(selected.name, 'python3', 'Should find python3 kernel'); }); - test('Kernel selection: Should find environment-specific kernel when it exists', () => { - const envId = 'my-env'; + test('Kernel selection: Should fall back to first available kernel when no Python kernel exists', () => { const kernelSpecs: IJupyterKernelSpec[] = [ - createMockKernelSpec('python3', 'Python 3', 'python'), - createMockKernelSpec(`deepnote-${envId}`, 'My Environment', 'python') + createMockKernelSpec('javascript', 'JavaScript', 'javascript'), + createMockKernelSpec('r', 'R', 'r') ]; - const selected = selector.selectKernelSpec(kernelSpecs, envId); + const selected = selector.selectKernelSpec(kernelSpecs); - assert.strictEqual(selected?.name, `deepnote-${envId}`); + assert.strictEqual(selected.name, 'javascript', 'Should fall back to first available kernel'); }); - test('Kernel selection: Should fall back to python3 when env kernel missing', () => { - // Documents current fallback behavior - falls back to python3 when env kernel missing - const envId = 'my-env'; - const kernelSpecs: IJupyterKernelSpec[] = [ - createMockKernelSpec('python3', 'Python 3', 'python'), - createMockKernelSpec('javascript', 'JavaScript', 'javascript') - ]; + test('Kernel selection: Should throw when no kernel specs are available', () => { + const kernelSpecs: IJupyterKernelSpec[] = []; - // Should fall back to python3 (current behavior) - const selected = selector.selectKernelSpec(kernelSpecs, envId); - - assert.strictEqual(selected.name, 'python3', 'Should fall back to python3'); + assert.throws( + () => selector.selectKernelSpec(kernelSpecs), + /No kernel specs available/, + 'Should throw when no kernel specs are available' + ); }); }); @@ -834,9 +729,14 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { // REAL TEST: This will FAIL if disposal happens too early // // Setup: Create a scenario where we have an old controller and create a new one - const baseFileUri = mockNotebook.uri.with({ query: '', fragment: '' }); + // const baseFileUri = mockNotebook.uri.with({ query: '', fragment: '' }); // const notebookKey = baseFileUri.fsPath; - const newEnv = createMockEnvironment('env-new', 'New Environment', true); + // Mock interpreter service to return an active interpreter + const mockInterpreter: PythonEnvironment = { + id: '/usr/bin/python3', + uri: Uri.parse('/usr/bin/python3') + }; + when(mockInterpreterService.getActiveInterpreter(anything())).thenResolve(mockInterpreter); // Track call order const callOrder: string[] = []; @@ -860,8 +760,6 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { when(newController.controller).thenReturn({} as any); // Setup mocks - when(mockNotebookEnvironmentMapper.getEnvironmentForNotebook(baseFileUri)).thenReturn('env-new'); - when(mockEnvironmentManager.getEnvironment('env-new')).thenReturn(newEnv); when(mockPythonExtensionChecker.isPythonExtensionInstalled).thenReturn(true); // Mock controller registration to track when new controller is added @@ -1019,37 +917,6 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { }); }); -/** - * Helper function to create mock environments - */ -function createMockEnvironment(id: string, name: string, hasServer: boolean = false): DeepnoteEnvironment { - const mockPythonInterpreter: PythonEnvironment = { - id: `/usr/bin/python3`, - uri: Uri.parse(`/usr/bin/python3`) - }; - - return { - id, - name, - description: `Test environment ${name}`, - pythonInterpreter: mockPythonInterpreter, - venvPath: Uri.file(`/test/venvs/${id}`), - managedVenv: true, - packages: [], - createdAt: new Date(), - lastUsedAt: new Date(), - serverInfo: hasServer - ? { - url: `http://localhost:8888`, - jupyterPort: 8888, - lspPort: 8889, - token: 'test-token', - process: createMockChildProcess() - } - : undefined - }; -} - /** * Helper function to create mock kernel specs */ diff --git a/src/platform/interpreter/installer/pipInstaller.node.ts b/src/platform/interpreter/installer/pipInstaller.node.ts index ae53dde627..6548baf90e 100644 --- a/src/platform/interpreter/installer/pipInstaller.node.ts +++ b/src/platform/interpreter/installer/pipInstaller.node.ts @@ -15,6 +15,8 @@ import { Environment } from '@vscode/python-extension'; import { getEnvironmentType } from '../helpers'; import { workspace } from 'vscode'; +import { DEEPNOTE_TOOLKIT_VERSION } from '../../../kernels/deepnote/types'; + /** * Installer for pip. Default installer for most everything. */ @@ -85,8 +87,14 @@ export class PipInstaller extends ModuleInstaller { if (getEnvironmentType(interpreter) === EnvironmentType.Unknown) { args.push('--user'); } + // deepnote_toolkit's import name differs from the pip package name (deepnote-toolkit[server]) + const pipPackageName = + moduleName === translateProductToModule(Product.deepnoteToolkit) + ? `deepnote-toolkit[server]==${DEEPNOTE_TOOLKIT_VERSION}` + : moduleName; + return { - args: ['-m', 'pip', ...args, moduleName].concat(getPinnedPackages('pip', moduleName)) + args: ['-m', 'pip', ...args, pipPackageName].concat(getPinnedPackages('pip', moduleName)) }; } private isPipAvailable(interpreter: PythonEnvironment | Environment): Promise { diff --git a/src/platform/interpreter/installer/productInstaller.node.ts b/src/platform/interpreter/installer/productInstaller.node.ts index bf59cc22f3..9390224f68 100644 --- a/src/platform/interpreter/installer/productInstaller.node.ts +++ b/src/platform/interpreter/installer/productInstaller.node.ts @@ -7,10 +7,12 @@ import { ProductNames } from './productNames'; import { IInstallationChannelManager, IInstaller, + IModuleInstaller, InstallerResponse, IProductPathService, IProductService, ModuleInstallFlags, + ModuleInstallerType, Product, ProductType } from './types'; @@ -93,8 +95,19 @@ export class DataScienceInstaller { installPipIfRequired?: boolean, silent?: boolean ): Promise { - const channels = this.serviceContainer.get(IInstallationChannelManager); - const installer = await channels.getInstallationChannel(product, interpreter); + let installer: IModuleInstaller | undefined; + + // deepnote-toolkit is PyPI-only with pip-specific [server] extras syntax, + // so always use PipInstaller regardless of environment type. + if (product === Product.deepnoteToolkit) { + const channels = this.serviceContainer.get(IInstallationChannelManager); + const allInstallers = await channels.getInstallationChannels(interpreter); + installer = allInstallers.find((i) => i.type === ModuleInstallerType.Pip); + } else { + const channels = this.serviceContainer.get(IInstallationChannelManager); + installer = await channels.getInstallationChannel(product, interpreter); + } + if (!installer) { return InstallerResponse.Ignore; } diff --git a/src/platform/interpreter/installer/productNames.ts b/src/platform/interpreter/installer/productNames.ts index 5c9eaa559b..246432faa1 100644 --- a/src/platform/interpreter/installer/productNames.ts +++ b/src/platform/interpreter/installer/productNames.ts @@ -12,3 +12,4 @@ ProductNames.set(Product.kernelspec, 'kernelspec'); ProductNames.set(Product.pandas, 'pandas'); ProductNames.set(Product.pip, 'pip'); ProductNames.set(Product.ensurepip, 'ensurepip'); +ProductNames.set(Product.deepnoteToolkit, 'deepnote-toolkit'); diff --git a/src/platform/interpreter/installer/productService.node.ts b/src/platform/interpreter/installer/productService.node.ts index 5059e2b2a0..8af4dfb549 100644 --- a/src/platform/interpreter/installer/productService.node.ts +++ b/src/platform/interpreter/installer/productService.node.ts @@ -20,6 +20,7 @@ export class ProductService implements IProductService { this.ProductTypes.set(Product.pandas, ProductType.DataScience); this.ProductTypes.set(Product.pip, ProductType.DataScience); this.ProductTypes.set(Product.ensurepip, ProductType.DataScience); + this.ProductTypes.set(Product.deepnoteToolkit, ProductType.DataScience); } public getProductType(product: Product): ProductType { return this.ProductTypes.get(product)!; diff --git a/src/platform/interpreter/installer/utils.ts b/src/platform/interpreter/installer/utils.ts index b85bd38d76..2e80f6d481 100644 --- a/src/platform/interpreter/installer/utils.ts +++ b/src/platform/interpreter/installer/utils.ts @@ -23,6 +23,8 @@ export function translateProductToModule(product: Product): string { return 'pip'; case Product.ensurepip: return 'ensurepip'; + case Product.deepnoteToolkit: + return 'deepnote_toolkit'; default: { throw new WrappedError( `Product ${product} cannot be installed as a Python Module.`, From 001526448f69c3ac9cd80fde4453fde7bd42e9ab Mon Sep 17 00:00:00 2001 From: tomas Date: Thu, 2 Apr 2026 06:01:35 +0000 Subject: [PATCH 2/7] refactor(deepnote): Enhance deepnote server management and cleanup logic - Updated DeepnoteServerStarter to improve context management and error handling during server startup. - Refactored cancellation token handling to ensure proper disposal and prevent memory leaks. - Enhanced logging for notebook closure to include cleanup of associated metadata. - Added unit tests for controller unselection logic to ensure correct behavior with Deepnote kernels. --- .../deepnote/deepnoteServerStarter.node.ts | 18 ++++--- .../deepnoteKernelAutoSelector.node.ts | 28 +++++----- ...epnoteKernelAutoSelector.node.unit.test.ts | 52 +++++++++++++++++++ src/platform/interpreter/installer/types.ts | 3 +- 4 files changed, 79 insertions(+), 22 deletions(-) diff --git a/src/kernels/deepnote/deepnoteServerStarter.node.ts b/src/kernels/deepnote/deepnoteServerStarter.node.ts index ff2c7e2696..ce2d442b6b 100644 --- a/src/kernels/deepnote/deepnoteServerStarter.node.ts +++ b/src/kernels/deepnote/deepnoteServerStarter.node.ts @@ -8,7 +8,7 @@ import * as fs from 'fs-extra'; import { inject, injectable, named, optional } from 'inversify'; import * as os from 'os'; -import { CancellationToken, l10n, Uri } from 'vscode'; +import { CancellationToken, CancellationTokenSource, l10n, Uri } from 'vscode'; import { startServer, stopServer } from '@deepnote/runtime-core'; @@ -138,7 +138,8 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension `Stopping existing server for ${fileKey} with interpreter ${existingInterpreterId} to start new one with interpreter ${interpreterId}...` ); await this.stopServerForEnvironment(existingContext, deepnoteFileUri, token); - existingContext.interpreterId = interpreterId; + existingContext = { interpreterId, serverInfo: null }; + this.projectContexts.set(fileKey, existingContext); } } else { const newContext: ProjectContext = { @@ -235,20 +236,23 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension if (!isInstalled) { logger.info(`deepnote-toolkit not installed, installing via IInstaller...`); - const { CancellationTokenSource } = await import('vscode'); const cts = new CancellationTokenSource(); + let cancellationListener: IDisposable | undefined; try { if (token) { - token.onCancellationRequested(() => cts.cancel()); + cancellationListener = token.onCancellationRequested(() => cts.cancel()); } const result = await this.installer.install(Product.deepnoteToolkit, interpreter, cts); - if (result !== InstallerResponse.Installed) { - throw new Error('deepnote-toolkit installation was cancelled or failed'); + if (result === InstallerResponse.Cancelled) { + throw new Error('deepnote-toolkit installation was cancelled by the user'); + } else if (result !== InstallerResponse.Installed) { + throw new Error('Failed to install deepnote-toolkit. Check the Output panel for details.'); } } finally { + cancellationListener?.dispose(); cts.dispose(); } } @@ -282,7 +286,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension throw new DeepnoteServerStartupError( interpreter.uri.fsPath, - serverInfo?.jupyterPort ?? 0, + 0, 'unknown', capturedOutput?.stdout || '', capturedOutput?.stderr || '', diff --git a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts index e7988635ee..cd2abd3119 100644 --- a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts +++ b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts @@ -203,14 +203,16 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, } private onDidCloseNotebook(notebook: NotebookDocument) { - logger.info(`Notebook closed: ${getDisplayPath(notebook.uri)}, with type: ${notebook.notebookType}`); - - // Only handle deepnote notebooks if (notebook.notebookType !== DEEPNOTE_NOTEBOOK_TYPE) { return; } - logger.info(`Deepnote notebook closed: ${getDisplayPath(notebook.uri)}`); + const notebookKey = notebook.uri.toString(); + this.notebookConnectionMetadata.delete(notebookKey); + this.notebookInterpreterIds.delete(notebookKey); + this.notebookControllers.delete(notebookKey); + + logger.info(`Deepnote notebook closed, cleaned up: ${getDisplayPath(notebook.uri)}`); } public async onKernelStarted(kernel: IKernel) { @@ -663,22 +665,20 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, /** * Clear the controller selection for a notebook using a specific environment. * This is used when deleting an environment to unselect its controller from any open notebooks. + * + * Since the refactoring, server handles are keyed by interpreter.id (not environmentId). + * We match by checking if the currently selected controller is one of ours (a Deepnote kernel + * controller), rather than reconstructing a handle from the environmentId. */ - public clearControllerForEnvironment(notebook: NotebookDocument, environmentId: string): void { + public clearControllerForEnvironment(notebook: NotebookDocument, _environmentId: string): void { const selectedController = this.controllerRegistration.getSelected(notebook); if (!selectedController || selectedController.connection.kind !== 'startUsingDeepnoteKernel') { return; } - const expectedHandle = createDeepnoteServerConfigHandle(environmentId, notebook.uri); - - if (selectedController.connection.serverProviderHandle.handle === expectedHandle) { - // Unselect the controller by setting affinity to Default - selectedController.controller.updateNotebookAffinity(notebook, NotebookControllerAffinity.Default); - logger.info( - `Cleared controller for notebook ${getDisplayPath(notebook.uri)} (environment ${environmentId})` - ); - } + // The selected controller is a Deepnote kernel — unselect it + selectedController.controller.updateNotebookAffinity(notebook, NotebookControllerAffinity.Default); + logger.info(`Cleared Deepnote controller for notebook ${getDisplayPath(notebook.uri)}`); } /** diff --git a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts index cb0b67ee69..b2d60dd1fb 100644 --- a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts @@ -915,6 +915,58 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { }); }); }); + + suite('clearControllerForEnvironment', () => { + test('should unselect controller when a Deepnote kernel is selected', () => { + const mockSelectedController = mock(); + when(mockSelectedController.connection).thenReturn({ + kind: 'startUsingDeepnoteKernel', + serverProviderHandle: { handle: 'some-handle' } + } as any); + + const mockNativeController = { + updateNotebookAffinity: sandbox.stub() + } as unknown as NotebookController; + when(mockSelectedController.controller).thenReturn(mockNativeController); + + when(mockControllerRegistration.getSelected(mockNotebook)).thenReturn(instance(mockSelectedController)); + + selector.clearControllerForEnvironment(mockNotebook, 'any-environment-id'); + + assert.isTrue( + (mockNativeController.updateNotebookAffinity as sinon.SinonStub).calledOnce, + 'Should have called updateNotebookAffinity' + ); + }); + + test('should not unselect controller when no controller is selected', () => { + when(mockControllerRegistration.getSelected(mockNotebook)).thenReturn(undefined); + + // Should not throw + selector.clearControllerForEnvironment(mockNotebook, 'any-environment-id'); + }); + + test('should not unselect controller when selected controller is not a Deepnote kernel', () => { + const mockSelectedController = mock(); + when(mockSelectedController.connection).thenReturn({ + kind: 'startUsingLocalKernelSpec' + } as any); + + const mockNativeController = { + updateNotebookAffinity: sandbox.stub() + } as unknown as NotebookController; + when(mockSelectedController.controller).thenReturn(mockNativeController); + + when(mockControllerRegistration.getSelected(mockNotebook)).thenReturn(instance(mockSelectedController)); + + selector.clearControllerForEnvironment(mockNotebook, 'any-environment-id'); + + assert.isFalse( + (mockNativeController.updateNotebookAffinity as sinon.SinonStub).called, + 'Should NOT have called updateNotebookAffinity for non-Deepnote kernel' + ); + }); + }); }); /** diff --git a/src/platform/interpreter/installer/types.ts b/src/platform/interpreter/installer/types.ts index bdb5bfee39..550886232c 100644 --- a/src/platform/interpreter/installer/types.ts +++ b/src/platform/interpreter/installer/types.ts @@ -21,7 +21,8 @@ export enum Product { nbconvert = 22, pandas = 23, pip = 27, - ensurepip = 28 + ensurepip = 28, + deepnoteToolkit = 29 } export enum ProductInstallStatus { From 774093732d1e3bc612ff1bbbdf02224858ea837d Mon Sep 17 00:00:00 2001 From: tomas Date: Thu, 2 Apr 2026 09:42:52 +0000 Subject: [PATCH 3/7] fix(deepnote): Improve controller management and cleanup logic - Enhanced the logic for clearing notebook controllers to ensure only tracked controllers are unselected. - Updated the `clearControllerForEnvironment` method to clean up associated metadata correctly. - Added unit tests to verify the behavior of environment configuration and controller unselection for Deepnote kernels. - Ensured that the system correctly handles cases where the active interpreter differs from the cached interpreter. --- .../deepnoteKernelAutoSelector.node.ts | 40 +++- ...epnoteKernelAutoSelector.node.unit.test.ts | 178 +++++++++++++++--- .../installer/productInstaller.node.ts | 5 +- .../installer/productInstaller.unit.test.ts | 59 ++++++ 4 files changed, 249 insertions(+), 33 deletions(-) diff --git a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts index cd2abd3119..e7196633ca 100644 --- a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts +++ b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts @@ -626,8 +626,9 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, } const existingController = this.notebookControllers.get(notebookKey); + const existingInterpreterId = this.notebookInterpreterIds.get(notebookKey); - if (existingController) { + if (existingController && existingInterpreterId === interpreter.id) { logger.info(`Controller already configured for ${getDisplayPath(notebook.uri)}`); return true; } @@ -663,22 +664,41 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, } /** - * Clear the controller selection for a notebook using a specific environment. - * This is used when deleting an environment to unselect its controller from any open notebooks. + * Clear the controller selection for a notebook if it was set up by this selector + * for the given environment. * - * Since the refactoring, server handles are keyed by interpreter.id (not environmentId). - * We match by checking if the currently selected controller is one of ours (a Deepnote kernel - * controller), rather than reconstructing a handle from the environmentId. + * The caller passes an `environmentId` (UUID), but the auto-selector now tracks + * notebooks by interpreter.id. We match by comparing the notebook's tracked + * controller instance against the currently selected controller, so we only + * clear controllers we own — never an unrelated Deepnote kernel. */ - public clearControllerForEnvironment(notebook: NotebookDocument, _environmentId: string): void { + public clearControllerForEnvironment(notebook: NotebookDocument, environmentId: string): void { + const notebookKey = notebook.uri.toString(); + const trackedController = this.notebookControllers.get(notebookKey); + + if (!trackedController) { + return; // We didn't set up a controller for this notebook + } + const selectedController = this.controllerRegistration.getSelected(notebook); - if (!selectedController || selectedController.connection.kind !== 'startUsingDeepnoteKernel') { + if (!selectedController || selectedController.id !== trackedController.id) { + return; // Selected controller isn't the one we own + } + + if (selectedController.connection.kind !== 'startUsingDeepnoteKernel') { return; } - // The selected controller is a Deepnote kernel — unselect it selectedController.controller.updateNotebookAffinity(notebook, NotebookControllerAffinity.Default); - logger.info(`Cleared Deepnote controller for notebook ${getDisplayPath(notebook.uri)}`); + + // Clean up our tracking state for this notebook + this.notebookControllers.delete(notebookKey); + this.notebookConnectionMetadata.delete(notebookKey); + this.notebookInterpreterIds.delete(notebookKey); + + logger.info( + `Cleared Deepnote controller for notebook ${getDisplayPath(notebook.uri)} (environment ${environmentId})` + ); } /** diff --git a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts index b2d60dd1fb..0f986b7aaa 100644 --- a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts @@ -372,6 +372,82 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { }); }); + suite('ensureEnvironmentConfiguredBeforeExecution', () => { + const nonCancelledToken: CancellationToken = { + isCancellationRequested: false, + onCancellationRequested: () => ({ dispose: () => {} }) as any + }; + + test('should reconfigure when active interpreter differs from cached interpreter', async () => { + const notebookKey = mockNotebook.uri.toString(); + const interpreterA: PythonEnvironment = { + id: '/usr/bin/python3.10', + uri: Uri.parse('/usr/bin/python3.10') + }; + const interpreterB: PythonEnvironment = { + id: '/usr/bin/python3.12', + uri: Uri.parse('/usr/bin/python3.12') + }; + + // Prime the internal maps: controller exists for interpreter A + const selectorAny = selector as any; + selectorAny.notebookControllers.set(notebookKey, instance(mockController)); + selectorAny.notebookInterpreterIds.set(notebookKey, interpreterA.id); + + // Active interpreter is now B + when(mockInterpreterService.getActiveInterpreter(anything())).thenResolve(interpreterB); + + // Stub ensureKernelSelectedWithInterpreter to track calls + const ensureStub = sandbox.stub(selector, 'ensureKernelSelectedWithInterpreter').resolves(); + + // withProgress must call through to the task callback + when(mockedVSCodeNamespaces.window.withProgress(anything(), anything())).thenCall( + (_opts: any, task: any) => { + return task({ report: sandbox.stub() }, nonCancelledToken); + } + ); + + // Put a controller in the map so the final check returns true + ensureStub.callsFake(async () => { + selectorAny.notebookControllers.set(notebookKey, instance(mockNewController)); + }); + + const result = await selector.ensureEnvironmentConfiguredBeforeExecution(mockNotebook, nonCancelledToken); + + assert.strictEqual(result, true, 'Should return true after reconfiguring'); + assert.strictEqual(ensureStub.calledOnce, true, 'Should call ensureKernelSelectedWithInterpreter'); + assert.deepStrictEqual( + ensureStub.firstCall.args[1], + interpreterB, + 'Should reconfigure with the new interpreter' + ); + }); + + test('should return true immediately when controller exists for the same interpreter', async () => { + const notebookKey = mockNotebook.uri.toString(); + const interpreterA: PythonEnvironment = { + id: '/usr/bin/python3.10', + uri: Uri.parse('/usr/bin/python3.10') + }; + + // Prime the internal maps: controller exists for interpreter A + const selectorAny = selector as any; + selectorAny.notebookControllers.set(notebookKey, instance(mockController)); + selectorAny.notebookInterpreterIds.set(notebookKey, interpreterA.id); + + // Active interpreter is still A + when(mockInterpreterService.getActiveInterpreter(anything())).thenResolve(interpreterA); + + // Stub ensureKernelSelectedWithInterpreter — should NOT be called + const ensureStub = sandbox.stub(selector, 'ensureKernelSelectedWithInterpreter').resolves(); + + const result = await selector.ensureEnvironmentConfiguredBeforeExecution(mockNotebook, nonCancelledToken); + + assert.strictEqual(result, true, 'Should return true (fast path)'); + assert.strictEqual(ensureStub.called, false, 'Should NOT call ensureKernelSelectedWithInterpreter'); + }); + }); + // Priority 1 Tests - Critical for environment switching // UT-4: Configuration Refresh After startServer suite('Priority 1: Configuration Refresh (UT-4)', () => { @@ -917,53 +993,113 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { }); suite('clearControllerForEnvironment', () => { - test('should unselect controller when a Deepnote kernel is selected', () => { - const mockSelectedController = mock(); - when(mockSelectedController.connection).thenReturn({ - kind: 'startUsingDeepnoteKernel', - serverProviderHandle: { handle: 'some-handle' } - } as any); + test('should unselect and clean up when tracked controller matches selected controller', () => { + const notebookKey = mockNotebook.uri.toString(); + // Set up a tracked controller in the internal map + const trackedController = mock(); + when(trackedController.id).thenReturn('deepnote-notebook-123'); + when(trackedController.connection).thenReturn({ + kind: 'startUsingDeepnoteKernel' + } as any); const mockNativeController = { updateNotebookAffinity: sandbox.stub() } as unknown as NotebookController; - when(mockSelectedController.controller).thenReturn(mockNativeController); + when(trackedController.controller).thenReturn(mockNativeController); + + const selectorAny = selector as any; + selectorAny.notebookControllers.set(notebookKey, instance(trackedController)); + selectorAny.notebookInterpreterIds.set(notebookKey, '/usr/bin/python3'); + selectorAny.notebookConnectionMetadata.set(notebookKey, {} as any); - when(mockControllerRegistration.getSelected(mockNotebook)).thenReturn(instance(mockSelectedController)); + // Selected controller is the same one we tracked + when(mockControllerRegistration.getSelected(mockNotebook)).thenReturn(instance(trackedController)); - selector.clearControllerForEnvironment(mockNotebook, 'any-environment-id'); + selector.clearControllerForEnvironment(mockNotebook, 'env-uuid-123'); assert.isTrue( (mockNativeController.updateNotebookAffinity as sinon.SinonStub).calledOnce, 'Should have called updateNotebookAffinity' ); + // Verify tracking state is cleaned up + assert.isFalse(selectorAny.notebookControllers.has(notebookKey), 'Should remove from notebookControllers'); + assert.isFalse( + selectorAny.notebookInterpreterIds.has(notebookKey), + 'Should remove from notebookInterpreterIds' + ); + assert.isFalse( + selectorAny.notebookConnectionMetadata.has(notebookKey), + 'Should remove from notebookConnectionMetadata' + ); + }); + + test('should NOT unselect when notebook has no tracked controller', () => { + // notebookControllers map is empty — we didn't set up this notebook + const trackedController = mock(); + const mockNativeController = { + updateNotebookAffinity: sandbox.stub() + } as unknown as NotebookController; + when(trackedController.controller).thenReturn(mockNativeController); + when(mockControllerRegistration.getSelected(mockNotebook)).thenReturn(instance(trackedController)); + + selector.clearControllerForEnvironment(mockNotebook, 'env-uuid-123'); + + assert.isFalse( + (mockNativeController.updateNotebookAffinity as sinon.SinonStub).called, + 'Should NOT have called updateNotebookAffinity when we have no tracked controller' + ); }); - test('should not unselect controller when no controller is selected', () => { - when(mockControllerRegistration.getSelected(mockNotebook)).thenReturn(undefined); + test('should NOT unselect when selected controller differs from tracked controller', () => { + const notebookKey = mockNotebook.uri.toString(); + + // Track controller A + const controllerA = mock(); + when(controllerA.id).thenReturn('deepnote-notebook-A'); + const selectorAny = selector as any; + selectorAny.notebookControllers.set(notebookKey, instance(controllerA)); + + // But VS Code has controller B selected (different id) + const controllerB = mock(); + when(controllerB.id).thenReturn('deepnote-notebook-B'); + const mockNativeController = { + updateNotebookAffinity: sandbox.stub() + } as unknown as NotebookController; + when(controllerB.controller).thenReturn(mockNativeController); + when(mockControllerRegistration.getSelected(mockNotebook)).thenReturn(instance(controllerB)); + + selector.clearControllerForEnvironment(mockNotebook, 'env-uuid-123'); - // Should not throw - selector.clearControllerForEnvironment(mockNotebook, 'any-environment-id'); + assert.isFalse( + (mockNativeController.updateNotebookAffinity as sinon.SinonStub).called, + 'Should NOT unselect a controller we do not own' + ); }); - test('should not unselect controller when selected controller is not a Deepnote kernel', () => { - const mockSelectedController = mock(); - when(mockSelectedController.connection).thenReturn({ + test('should NOT unselect when selected controller is not a Deepnote kernel', () => { + const notebookKey = mockNotebook.uri.toString(); + + // Track a controller + const trackedController = mock(); + when(trackedController.id).thenReturn('deepnote-notebook-123'); + when(trackedController.connection).thenReturn({ kind: 'startUsingLocalKernelSpec' } as any); - const mockNativeController = { updateNotebookAffinity: sandbox.stub() } as unknown as NotebookController; - when(mockSelectedController.controller).thenReturn(mockNativeController); + when(trackedController.controller).thenReturn(mockNativeController); + + const selectorAny = selector as any; + selectorAny.notebookControllers.set(notebookKey, instance(trackedController)); - when(mockControllerRegistration.getSelected(mockNotebook)).thenReturn(instance(mockSelectedController)); + when(mockControllerRegistration.getSelected(mockNotebook)).thenReturn(instance(trackedController)); - selector.clearControllerForEnvironment(mockNotebook, 'any-environment-id'); + selector.clearControllerForEnvironment(mockNotebook, 'env-uuid-123'); assert.isFalse( (mockNativeController.updateNotebookAffinity as sinon.SinonStub).called, - 'Should NOT have called updateNotebookAffinity for non-Deepnote kernel' + 'Should NOT unselect a non-Deepnote kernel' ); }); }); diff --git a/src/platform/interpreter/installer/productInstaller.node.ts b/src/platform/interpreter/installer/productInstaller.node.ts index 9390224f68..e9e8dac97a 100644 --- a/src/platform/interpreter/installer/productInstaller.node.ts +++ b/src/platform/interpreter/installer/productInstaller.node.ts @@ -99,9 +99,10 @@ export class DataScienceInstaller { // deepnote-toolkit is PyPI-only with pip-specific [server] extras syntax, // so always use PipInstaller regardless of environment type. + // We bypass getInstallationChannels() because it filters by isSupported(), + // and PipInstaller.isSupported() rejects Conda/Pipenv/Poetry interpreters. if (product === Product.deepnoteToolkit) { - const channels = this.serviceContainer.get(IInstallationChannelManager); - const allInstallers = await channels.getInstallationChannels(interpreter); + const allInstallers = this.serviceContainer.getAll(IModuleInstaller); installer = allInstallers.find((i) => i.type === ModuleInstallerType.Pip); } else { const channels = this.serviceContainer.get(IInstallationChannelManager); diff --git a/src/platform/interpreter/installer/productInstaller.unit.test.ts b/src/platform/interpreter/installer/productInstaller.unit.test.ts index dbe9dba1db..27daf48151 100644 --- a/src/platform/interpreter/installer/productInstaller.unit.test.ts +++ b/src/platform/interpreter/installer/productInstaller.unit.test.ts @@ -206,4 +206,63 @@ suite('DataScienceInstaller install', async () => { const result = await dataScienceInstaller.install(Product.ipykernel, testEnvironment, tokenSource); expect(result).to.equal(InstallerResponse.Installed, 'Should be Installed'); }); + + test('Will use pip for deepnoteToolkit even on Conda interpreter (bypasses isSupported filter)', async () => { + const testEnvironment: PythonEnvironment = { + id: interpreterPath.fsPath, + uri: interpreterPath + }; + + // Create a pip installer mock + const pipInstaller = TypeMoq.Mock.ofType(); + pipInstaller.setup((c) => c.type).returns(() => ModuleInstallerType.Pip); + pipInstaller + .setup((c) => + c.installModule( + TypeMoq.It.isValue(Product.deepnoteToolkit), + TypeMoq.It.isValue(testEnvironment), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny() + ) + ) + .returns(() => Promise.resolve()); + pipInstaller.setup((p) => (p as any).then).returns(() => undefined); + + // Create a conda installer mock (would normally be selected for Conda envs) + const condaInstaller = TypeMoq.Mock.ofType(); + condaInstaller.setup((c) => c.type).returns(() => ModuleInstallerType.Conda); + + // serviceContainer.getAll returns both installers — the code must pick pip + serviceContainer + .setup((c) => c.getAll(TypeMoq.It.isValue(IModuleInstaller))) + .returns(() => [condaInstaller.object, pipInstaller.object]); + + const result = await dataScienceInstaller.install(Product.deepnoteToolkit, testEnvironment, tokenSource); + expect(result).to.equal(InstallerResponse.Installed, 'Should be Installed via pip'); + + // Verify pip was called, not conda + pipInstaller.verify( + (c) => + c.installModule( + TypeMoq.It.isValue(Product.deepnoteToolkit), + TypeMoq.It.isValue(testEnvironment), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny() + ), + TypeMoq.Times.once() + ); + condaInstaller.verify( + (c) => + c.installModule( + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny(), + TypeMoq.It.isAny() + ), + TypeMoq.Times.never() + ); + }); }); From 3dd23fe12f98bb8c27186f637da8a803c071d739 Mon Sep 17 00:00:00 2001 From: tomas Date: Mon, 24 Aug 2026 19:49:56 +0000 Subject: [PATCH 4/7] fix(python-api): don't hang forever waiting for the Jupyter API handshake Opening any .deepnote notebook hung at "Starting Deepnote server..." and the kernel never started. DeepnoteServerStarter now checks the toolkit through IInstaller, which reaches IEnvironmentActivationService -> IPythonApiProvider.getApi(). That promise is resolved by `registerPythonApi`, which the Python extension only calls on the extension it knows as `ms-toolsai.jupyter`; this fork ships as `Deepnote.vscode-deepnote`, so the callback never arrives. getApi() is awaited with no token and no timeout, so isInstalled() never returned. The environment based flow never touched this path, which is why it only surfaces now. Fail the promise once the handshake has clearly not landed. Callers already handle it: getActivatedEnvironmentVariablesImpl catches and returns undefined, and createActivatedEnvironment then falls back to unactivated execution. Add an E2E test for the environment-free flow: a workspace whose active interpreter is a bare venv, so opening the notebook installs deepnote-toolkit into that interpreter and runs the cell. The cell prints sys.prefix, so the output proves the kernel ran in that venv rather than a Deepnote-managed environment. Asserts on the venv contents rather than the transient install toast, which is missed on a retry. Verified: E2E test fails (install never runs) without the fix and passes with it; typecheck 0, 2761 unit tests passing, 0 failing. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019cqzx43mLQRvVfWbfEUUSx --- src/platform/api/pythonApi.ts | 24 +++ test/e2e/fixtures/interpreter-kernel.deepnote | 23 +++ test/e2e/helpers/notifications.ts | 27 +++ test/e2e/suite/interpreterKernel.e2e.test.ts | 163 ++++++++++++++++++ 4 files changed, 237 insertions(+) create mode 100644 test/e2e/fixtures/interpreter-kernel.deepnote create mode 100644 test/e2e/suite/interpreterKernel.e2e.test.ts diff --git a/src/platform/api/pythonApi.ts b/src/platform/api/pythonApi.ts index 3e68cc60f6..18e620063f 100644 --- a/src/platform/api/pythonApi.ts +++ b/src/platform/api/pythonApi.ts @@ -49,6 +49,10 @@ import { trackInterpreterDiscovery, trackPythonExtensionActivation } from '../.. import { findPythonEnvBelongingToFolder } from '../../notebooks/controllers/preferredKernelConnectionService.node'; import { DisposableMap } from '../common/utils/lifecycle'; +// The Python extension completes its Jupyter handshake asynchronously; if the callback has not +// landed by then it never will (see failApiIfHandshakeNeverLands). +const PYTHON_API_HANDSHAKE_TIMEOUT = 5_000; + export function deserializePythonEnvironment( pythonVersion: Partial | undefined, pythonEnvId: string @@ -161,6 +165,25 @@ export class OldPythonApiProvider implements IPythonApiProvider { return extension?.exports; } + /** + * The Python extension ends the handshake by calling `registerPythonApi` on the extension it + * knows as `ms-toolsai.jupyter`. This fork ships under a different id, so that callback never + * arrives and every `getApi()` awaiter would hang forever. Fail the promise instead — callers + * already degrade to unactivated execution when activation variables are unavailable. + */ + private failApiIfHandshakeNeverLands() { + const timer = setTimeout(() => { + if (!this.api.resolved && !this.api.rejected) { + logger.warn('Python extension did not complete the Jupyter API handshake; continuing without it'); + this.api.reject(new PythonExtensionApiNotExportedError()); + } + }, PYTHON_API_HANDSHAKE_TIMEOUT); + + // Nothing may be awaiting the promise at rejection time. + this.api.promise.catch(noop); + this.disposables.push({ dispose: () => clearTimeout(timer) }); + } + public setApi(api: PythonApi): void { // Never allow accessing python API (we don't want to ever use the API and run code in untrusted API). // Don't assume Python API will always be disabled in untrusted workspaces. @@ -219,6 +242,7 @@ export class OldPythonApiProvider implements IPythonApiProvider { this.api.reject(new PythonExtensionApiNotExportedError()); } else { pythonExtension.exports.jupyter.registerHooks(); + this.failApiIfHandshakeNeverLands(); } this._pythonExtensionHooked.resolve(); } diff --git a/test/e2e/fixtures/interpreter-kernel.deepnote b/test/e2e/fixtures/interpreter-kernel.deepnote new file mode 100644 index 0000000000..ecb1a47e88 --- /dev/null +++ b/test/e2e/fixtures/interpreter-kernel.deepnote @@ -0,0 +1,23 @@ +version: '1.0.0' +metadata: + createdAt: '2025-01-01T00:00:00.000Z' + modifiedAt: '2025-01-01T00:00:00.000Z' +project: + id: e2e-interpreter-kernel-project + name: E2E Interpreter Kernel + notebooks: + - id: e2e-interpreter-kernel-notebook + name: Interpreter Kernel + blocks: + - id: e2e-interpreter-kernel-block + blockGroup: e2e-interpreter-kernel-group + type: code + content: |- + import sys + print("interpreter-kernel-ok") + print(sys.prefix) + sortingKey: a0 + metadata: {} + executionMode: block + isModule: false + settings: {} diff --git a/test/e2e/helpers/notifications.ts b/test/e2e/helpers/notifications.ts index 7f5ccadded..b5d3ca23c4 100644 --- a/test/e2e/helpers/notifications.ts +++ b/test/e2e/helpers/notifications.ts @@ -60,3 +60,30 @@ export async function waitForNotification( return undefined; } } + +/** + * Waits until no visible notification matches `pattern` any more. A progress notification is + * removed when its operation settles, so this gates on "that work finished" rather than on a + * fixed sleep. + */ +export async function waitForNotificationToClear(pattern: RegExp, timeout: number): Promise { + await VSBrowser.instance.driver.wait( + async () => { + const notifications = await new Workbench().getNotifications().catch((error) => { + console.warn('[deepnote-e2e] get notifications:', error); + + return [] as Notification[]; + }); + for (const notification of notifications) { + const message = await notification.getMessage().catch(() => ''); + if (pattern.test(message)) { + return false; + } + } + + return true; + }, + timeout, + `timed out waiting for notifications matching ${pattern} to clear` + ); +} diff --git a/test/e2e/suite/interpreterKernel.e2e.test.ts b/test/e2e/suite/interpreterKernel.e2e.test.ts new file mode 100644 index 0000000000..156adb317b --- /dev/null +++ b/test/e2e/suite/interpreterKernel.e2e.test.ts @@ -0,0 +1,163 @@ +/** + * End-to-end UI test for kernel setup WITHOUT Deepnote environments. + * + * This is the flow the extension uses now that environments are gone: the kernel is built from the + * workspace's *active Python interpreter*, and deepnote-toolkit is installed into that interpreter + * through the Python extension's installer infrastructure (`IInstaller`) — the same mechanism the + * Jupyter extension uses for its own missing dependencies: + * 1. open a one-notebook `.deepnote` file against a workspace whose active interpreter is a bare venv + * 2. the auto-selector picks that interpreter — no environment is created and no picker appears + * 3. deepnote-toolkit is missing, so an "Installing deepnote-toolkit" progress notification shows + * 4. once it installs, the server starts and the kernel controller is bound + * 5. run the cell and assert the rendered stdout + * + * The cell prints `sys.prefix`, so the output proves the kernel really ran inside the venv this test + * created rather than in a Deepnote-managed environment. The suite also asserts the toolkit landed in + * that same venv, which is the load-bearing difference from the environment-based flow. + * + * Prerequisites: + * - The Python extension (`ms-python.python`) must be installed in the test instance + * (`npm run setup:e2e:deps`). + * - `python3` must be on PATH and able to create a venv (CI installs `python3.12-venv`). + * - Network access: the toolkit is installed from PyPI on first kernel start, which is slow. + */ + +import { expect } from 'chai'; +import { execFileSync } from 'child_process'; +import * as fs from 'fs'; +import * as path from 'path'; +import { EditorView, VSBrowser, WebView } from 'vscode-extension-tester'; + +import { + FIRST_RUN_OUTPUT_TIMEOUT, + KERNEL_CONNECT_TIMEOUT, + SUITE_TIMEOUT, + WORKBENCH_TIMEOUT, + copyFixtureToTempDir, + openFolderViaDialog, + openWorkspaceFile, + runOnceAndAwaitOutput, + waitForNotification, + waitForNotificationToClear +} from '../helpers'; + +const NOTEBOOK_FILE_NAME = 'interpreter-kernel.deepnote'; +const EXPECTED_OUTPUT = 'interpreter-kernel-ok'; + +// The install toast is observed opportunistically; the venv check below is the real gate, so a +// missed toast must not cost the suite a full kernel-connect timeout. +const TOOLKIT_NOTIFICATION_TIMEOUT = 90_000; + +/** Path to the interpreter inside a venv, for the platform the test is running on. */ +function venvPython(venvDir: string): string { + return process.platform === 'win32' + ? path.join(venvDir, 'Scripts', 'python.exe') + : path.join(venvDir, 'bin', 'python'); +} + +/** True when `deepnote_toolkit` imports in the given interpreter. */ +function isToolkitInstalled(python: string): boolean { + try { + execFileSync(python, ['-c', 'import deepnote_toolkit'], { stdio: 'ignore' }); + + return true; + } catch { + return false; + } +} + +describe('Deepnote E2E — run on the active interpreter (no Deepnote environment)', function () { + this.timeout(SUITE_TIMEOUT); + + let cleanupTempDir: (() => void) | undefined; + let venvDir: string; + let interpreter: string; + + before(async function () { + const { cleanup, tempDir } = copyFixtureToTempDir(NOTEBOOK_FILE_NAME); + cleanupTempDir = cleanup; + + // A throwaway venv is what makes this test deterministic: it is guaranteed not to have + // deepnote-toolkit, so the install path runs on every execution rather than only on a + // machine that happens to be missing the package. + venvDir = path.join(tempDir, '.venv'); + execFileSync('python3', ['-m', 'venv', venvDir], { stdio: 'inherit' }); + interpreter = venvPython(venvDir); + + expect(isToolkitInstalled(interpreter)).to.equal( + false, + 'precondition: the fresh venv must not already provide deepnote-toolkit' + ); + + // Pin the workspace's interpreter so the auto-selector resolves this venv and not whatever + // the Python extension would otherwise discover on the machine. + const vscodeDir = path.join(tempDir, '.vscode'); + fs.mkdirSync(vscodeDir, { recursive: true }); + fs.writeFileSync( + path.join(vscodeDir, 'settings.json'), + JSON.stringify({ 'python.defaultInterpreterPath': interpreter }, undefined, 4) + ); + + await VSBrowser.instance.waitForWorkbench(WORKBENCH_TIMEOUT); + + // Opening the folder reloads the window, so the notebook is opened in the test body — that + // keeps the install notification, which fires during the open, inside the assertions. + await openFolderViaDialog(tempDir); + await VSBrowser.instance.waitForWorkbench(WORKBENCH_TIMEOUT); + }); + + after(async function () { + await new WebView().switchBack().catch((error) => { + console.warn('[deepnote-e2e] switch back from webview during cleanup:', error); + }); + await new EditorView().closeAllEditors().catch((error) => { + console.warn('[deepnote-e2e] close all editors during cleanup:', error); + }); + + try { + cleanupTempDir?.(); + } catch (error) { + console.warn('[deepnote-e2e] remove temp workspace dir during cleanup:', error); + } + }); + + it('installs deepnote-toolkit into the active interpreter, then runs the cell', async function () { + await openWorkspaceFile(NOTEBOOK_FILE_NAME); + + await VSBrowser.instance.driver.wait( + async () => (await new EditorView().getOpenEditorTitles()).some((t) => t.includes(NOTEBOOK_FILE_NAME)), + WORKBENCH_TIMEOUT, + 'Deepnote notebook editor did not open' + ); + + // Opening the notebook auto-selects the kernel, which finds the toolkit missing and installs + // it — no environment is created and the user is never asked to pick one. The progress toast + // is the visible half of that, but it is transient (and absent on a retry, where the toolkit + // is already installed), so it is observed best-effort; the durable assertions are below. + await waitForNotification(/Installing deepnote-toolkit/i, TOOLKIT_NOTIFICATION_TIMEOUT, false); + + // The load-bearing gate: the install landed in the active interpreter. Polling the venv is + // race-free, unlike matching a toast that may already have gone. + await VSBrowser.instance.driver.wait( + () => isToolkitInstalled(interpreter), + KERNEL_CONNECT_TIMEOUT, + 'deepnote-toolkit was never installed into the active interpreter' + ); + + // Waiting for the auto-select toast to be *gone* gates "Run All" on a bound kernel without + // depending on catching it while it is shown. + await waitForNotificationToClear(/Auto-selecting Deepnote kernel/i, KERNEL_CONNECT_TIMEOUT); + + const renderedOutput = await runOnceAndAwaitOutput( + NOTEBOOK_FILE_NAME, + EXPECTED_OUTPUT, + FIRST_RUN_OUTPUT_TIMEOUT + ); + + expect(renderedOutput).to.contain(EXPECTED_OUTPUT); + + // The cell printed sys.prefix: the kernel must be the venv this test created, which is what + // separates "active interpreter" from the old Deepnote-managed environment. + expect(renderedOutput).to.contain(venvDir); + }); +}); From 92e055f7724a53d7cb7f3cb13875154b6d154cc7 Mon Sep 17 00:00:00 2001 From: tomas Date: Tue, 25 Aug 2026 17:40:15 +0000 Subject: [PATCH 5/7] feat(deepnote): ask before installing the toolkit, and check at kernel start Brings the toolkit install in line with how the Jupyter extension handles a missing Python dependency. Consent. The install no longer runs unattended. DeepnoteToolkitDependencyService mirrors KernelDependencyService: the same modal message ("Running cells with '{env}' requires the {pkg} package."), Install as the default, and a "Select a different Interpreter" escape hatch for users who do not want the package in the interpreter that happens to be active. It cannot reuse that service directly because installMissingDependencies is keyed on a KernelConnectionMetadata, and a Deepnote connection cannot exist until the toolkit server is running -- which is what the check gates. Kernel start, not notebook open. Opening a .deepnote file now only offers a placeholder controller; nothing is installed and no server starts. Running a cell performs the check, the prompt, the install and the server start, then asks the user to re-run -- the same shape main used before environments were removed. Cancellation. Declining or cancelling aborts the kernel start and execution does not proceed, with no error dialog: a user-initiated stop is not a failure. An install that runs and does not take is still reported as a failure. The server starter no longer installs anything; it starts servers. Tests: unit coverage for each consent outcome (verified to fail when the prompt is bypassed). The E2E test now asserts that opening the notebook starts no install, drives the modal, and captures screenshots of the prompt, the kernel-ready state and the cell output. Verified: typecheck 0, 2767 unit tests passing, E2E green with the flow confirmed visually. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019cqzx43mLQRvVfWbfEUUSx --- .../deepnote/deepnoteServerStarter.node.ts | 32 +-- .../deepnoteServerStarter.unit.test.ts | 10 +- .../deepnoteToolkitDependencyService.node.ts | 103 +++++++++ ...pnoteToolkitDependencyService.unit.test.ts | 116 ++++++++++ src/kernels/deepnote/types.ts | 26 +++ .../deepnoteKernelAutoSelector.node.ts | 199 ++++++++++++++---- ...epnoteKernelAutoSelector.node.unit.test.ts | 9 +- src/notebooks/serviceRegistry.node.ts | 6 + src/platform/common/utils/localize.ts | 1 + test/e2e/helpers/modals.ts | 8 +- test/e2e/suite/interpreterKernel.e2e.test.ts | 105 +++++---- 11 files changed, 490 insertions(+), 125 deletions(-) create mode 100644 src/kernels/deepnote/deepnoteToolkitDependencyService.node.ts create mode 100644 src/kernels/deepnote/deepnoteToolkitDependencyService.unit.test.ts diff --git a/src/kernels/deepnote/deepnoteServerStarter.node.ts b/src/kernels/deepnote/deepnoteServerStarter.node.ts index 08fd6ebb67..1509ffae88 100644 --- a/src/kernels/deepnote/deepnoteServerStarter.node.ts +++ b/src/kernels/deepnote/deepnoteServerStarter.node.ts @@ -8,7 +8,7 @@ import * as fs from 'fs-extra'; import { inject, injectable, named } from 'inversify'; import * as os from 'os'; -import { CancellationToken, CancellationTokenSource, l10n, Uri } from 'vscode'; +import { CancellationToken, l10n, Uri } from 'vscode'; import { startServer, stopServer } from '@deepnote/runtime-core'; @@ -21,7 +21,6 @@ import { sleep } from '../../platform/common/utils/async'; import { generateUuid } from '../../platform/common/uuid'; import { DeepnoteServerStartupError } from '../../platform/errors/deepnoteKernelErrors'; import { getCachedEnvironment } from '../../platform/interpreter/helpers'; -import { IInstaller, InstallerResponse, Product } from '../../platform/interpreter/installer/types'; import { logger } from '../../platform/logging'; import { IUserpodApiEndpoints } from '../../platform/notebooks/deepnote/types'; import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; @@ -74,7 +73,6 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension constructor( @inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory, - @inject(IInstaller) private readonly installer: IInstaller, @inject(DeepnoteAgentSkillsManager) private readonly agentSkillsManager: DeepnoteAgentSkillsManager, @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly outputChannel: IOutputChannel, @inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry, @@ -214,7 +212,7 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension * Core server start using @deepnote/runtime-core's `startServer`. * * Extension-specific layers: - * - Toolkit check/install via IInstaller (before start) + * - The caller guarantees deepnote-toolkit is installed (IDeepnoteToolkitDependencyService) * - Integration endpoint env var injection (via ServerOptions.env) — these point the toolkit at the * extension's loopback `userpod-api` endpoint, which is how it fetches SQL credentials at kernel init * - Lock file creation (after start, using returned PID) @@ -232,32 +230,6 @@ export class DeepnoteServerStarter implements IDeepnoteServerStarter, IExtension Cancellation.throwIfCanceled(token); // Check if deepnote-toolkit is installed, and install if needed - logger.info(`Checking deepnote-toolkit installation for interpreter ${interpreterId}...`); - const isInstalled = await this.installer.isInstalled(Product.deepnoteToolkit, interpreter); - - if (!isInstalled) { - logger.info(`deepnote-toolkit not installed, installing via IInstaller...`); - const cts = new CancellationTokenSource(); - let cancellationListener: IDisposable | undefined; - - try { - if (token) { - cancellationListener = token.onCancellationRequested(() => cts.cancel()); - } - - const result = await this.installer.install(Product.deepnoteToolkit, interpreter, cts); - - if (result === InstallerResponse.Cancelled) { - throw new Error('deepnote-toolkit installation was cancelled by the user'); - } else if (result !== InstallerResponse.Installed) { - throw new Error('Failed to install deepnote-toolkit. Check the Output panel for details.'); - } - } finally { - cancellationListener?.dispose(); - cts.dispose(); - } - } - this.agentSkillsManager.ensureSkillsUpdated(interpreterId, interpreter); Cancellation.throwIfCanceled(token); diff --git a/src/kernels/deepnote/deepnoteServerStarter.unit.test.ts b/src/kernels/deepnote/deepnoteServerStarter.unit.test.ts index 4775e16153..7703150474 100644 --- a/src/kernels/deepnote/deepnoteServerStarter.unit.test.ts +++ b/src/kernels/deepnote/deepnoteServerStarter.unit.test.ts @@ -2,14 +2,13 @@ import { assert } from 'chai'; import * as fakeTimers from '@sinonjs/fake-timers'; import * as sinon from 'sinon'; import { anything, instance, mock, when } from 'ts-mockito'; -import { EventEmitter, Uri } from 'vscode'; +import { Uri } from 'vscode'; import { serializeProjectFile } from '../../notebooks/deepnote/deepnoteTestHelpers'; import { IProcessServiceFactory } from '../../platform/common/process/types.node'; import { IAsyncDisposableRegistry, IOutputChannel } from '../../platform/common/types'; import { PythonExtension } from '@vscode/python-extension'; import { setPythonApi } from '../../platform/interpreter/helpers'; -import { IInstaller, InstallerResponse } from '../../platform/interpreter/installer/types'; import { IUserpodApiEndpoints } from '../../platform/notebooks/deepnote/types'; import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; import { @@ -42,7 +41,6 @@ suite('DeepnoteServerStarter', () => { let serverStarter: DeepnoteServerStarter; let mockProcessServiceFactory: IProcessServiceFactory; - let mockInstaller: IInstaller; let mockAgentSkillsManager: DeepnoteAgentSkillsManager; let mockOutputChannel: IOutputChannel; let mockAsyncRegistry: IAsyncDisposableRegistry; @@ -53,7 +51,6 @@ suite('DeepnoteServerStarter', () => { resetVSCodeMocks(); mockProcessServiceFactory = mock(); - mockInstaller = mock(); mockAgentSkillsManager = mock(); mockOutputChannel = mock(); mockAsyncRegistry = mock(); @@ -61,10 +58,6 @@ suite('DeepnoteServerStarter', () => { when(mockAsyncRegistry.push(anything())).thenReturn(); when(mockOutputChannel.appendLine(anything())).thenReturn(); - when(mockInstaller.isInstalled(anything(), anything())).thenResolve(true); - when(mockInstaller.install(anything(), anything(), anything())).thenResolve(InstallerResponse.Installed); - when(mockInstaller.onInstalled).thenReturn(new EventEmitter().event); - when(mockUserpodApiEndpoints.ready).thenReturn(Promise.resolve()); when(mockUserpodApiEndpoints.baseUrl).thenReturn(undefined); @@ -80,7 +73,6 @@ suite('DeepnoteServerStarter', () => { serverStarter = new DeepnoteServerStarter( instance(mockProcessServiceFactory), - instance(mockInstaller), instance(mockAgentSkillsManager), instance(mockOutputChannel), instance(mockAsyncRegistry), diff --git a/src/kernels/deepnote/deepnoteToolkitDependencyService.node.ts b/src/kernels/deepnote/deepnoteToolkitDependencyService.node.ts new file mode 100644 index 0000000000..07cce6c341 --- /dev/null +++ b/src/kernels/deepnote/deepnoteToolkitDependencyService.node.ts @@ -0,0 +1,103 @@ +import { inject, injectable } from 'inversify'; +import { CancellationToken, CancellationTokenSource, commands, window } from 'vscode'; + +import { getDisplayPath } from '../../platform/common/platform/fs-paths.node'; +import { IDisposable, Resource } from '../../platform/common/types'; +import { Common, DataScience } from '../../platform/common/utils/localize'; +import { getPythonEnvDisplayName } from '../../platform/interpreter/helpers'; +import { ProductNames } from '../../platform/interpreter/installer/productNames'; +import { IInstaller, InstallerResponse, Product } from '../../platform/interpreter/installer/types'; +import { logger } from '../../platform/logging'; +import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; +import { DeepnoteToolkitDependencyResponse, IDeepnoteToolkitDependencyService } from './types'; + +const SELECT_INTERPRETER_COMMAND = 'python.setInterpreter'; + +/** + * Asks for consent before installing deepnote-toolkit into the user's interpreter, mirroring + * `KernelDependencyService` — same prompt shape, same "cancel is not a failure" semantics. + * + * It cannot reuse that service directly: `installMissingDependencies` is keyed on a + * `KernelConnectionMetadata`, and a Deepnote connection cannot exist until the toolkit server is + * running and has reported its kernelspecs — which is precisely what this check gates. + */ +@injectable() +export class DeepnoteToolkitDependencyService implements IDeepnoteToolkitDependencyService { + constructor(@inject(IInstaller) private readonly installer: IInstaller) {} + + public async ensureToolkitInstalled( + interpreter: PythonEnvironment, + resource: Resource, + token: CancellationToken + ): Promise { + if (await this.installer.isInstalled(Product.deepnoteToolkit, interpreter)) { + return DeepnoteToolkitDependencyResponse.ok; + } + + if (token.isCancellationRequested) { + return DeepnoteToolkitDependencyResponse.cancel; + } + + const moduleName = ProductNames.get(Product.deepnoteToolkit)!; + const message = DataScience.libraryRequiredToLaunchJupyterKernelNotInstalledInterpreter( + getPythonEnvDisplayName(interpreter) || getDisplayPath(interpreter.uri), + moduleName + ); + const selectInterpreter = DataScience.selectDifferentPythonInterpreter; + + logger.info(`${moduleName} missing for ${getDisplayPath(resource)}, prompting to install`); + + const selection = await window.showInformationMessage( + message, + { modal: true }, + Common.install, + selectInterpreter + ); + + if (selection === selectInterpreter) { + await commands.executeCommand(SELECT_INTERPRETER_COMMAND); + + return DeepnoteToolkitDependencyResponse.selectDifferentInterpreter; + } + + if (selection !== Common.install) { + logger.info(`User declined to install ${moduleName}`); + + return DeepnoteToolkitDependencyResponse.cancel; + } + + return this.install(interpreter, moduleName, token); + } + + private async install( + interpreter: PythonEnvironment, + moduleName: string, + token: CancellationToken + ): Promise { + const cts = new CancellationTokenSource(); + let cancellationListener: IDisposable | undefined; + + try { + cancellationListener = token.onCancellationRequested(() => cts.cancel()); + + const result = await this.installer.install(Product.deepnoteToolkit, interpreter, cts); + + if (result === InstallerResponse.Installed) { + return DeepnoteToolkitDependencyResponse.ok; + } + + if (result === InstallerResponse.Cancelled || token.isCancellationRequested) { + logger.info(`${moduleName} installation cancelled`); + + return DeepnoteToolkitDependencyResponse.cancel; + } + + logger.error(`${moduleName} installation did not complete: ${InstallerResponse[result]}`); + + return DeepnoteToolkitDependencyResponse.failed; + } finally { + cancellationListener?.dispose(); + cts.dispose(); + } + } +} diff --git a/src/kernels/deepnote/deepnoteToolkitDependencyService.unit.test.ts b/src/kernels/deepnote/deepnoteToolkitDependencyService.unit.test.ts new file mode 100644 index 0000000000..8fa7668999 --- /dev/null +++ b/src/kernels/deepnote/deepnoteToolkitDependencyService.unit.test.ts @@ -0,0 +1,116 @@ +import { assert } from 'chai'; +import { PythonExtension } from '@vscode/python-extension'; +import * as sinon from 'sinon'; +import { anything, instance, mock, verify, when } from 'ts-mockito'; +import { Uri } from 'vscode'; + +import { setPythonApi } from '../../platform/interpreter/helpers'; +import { resolvableInstance } from '../../test/datascience/helpers'; + +import { IInstaller, InstallerResponse, Product } from '../../platform/interpreter/installer/types'; +import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; +import { mockedVSCodeNamespaces, resetVSCodeMocks } from '../../test/vscode-mock'; +import { DeepnoteToolkitDependencyService } from './deepnoteToolkitDependencyService.node'; +import { DeepnoteToolkitDependencyResponse } from './types'; + +suite('DeepnoteToolkitDependencyService', () => { + const interpreter: PythonEnvironment = { + id: '/usr/bin/python3', + uri: Uri.file('/usr/bin/python3') + }; + const resource = Uri.file('/workspace/project/notebook.deepnote'); + const notCancelled = { isCancellationRequested: false, onCancellationRequested: () => ({ dispose: () => {} }) }; + + let installer: IInstaller; + let service: DeepnoteToolkitDependencyService; + + /** Makes the consent prompt resolve to `choice` (undefined = the user dismissed it). */ + function answerPrompt(choice: string | undefined) { + when( + mockedVSCodeNamespaces.window.showInformationMessage(anything(), anything(), anything(), anything()) + ).thenResolve(choice as never); + } + + setup(() => { + resetVSCodeMocks(); + installer = mock(); + service = new DeepnoteToolkitDependencyService(instance(installer)); + + // The prompt names the environment via getPythonEnvDisplayName, which reads the Python API. + const mockedApi = mock(); + sinon.stub(PythonExtension, 'api').resolves(resolvableInstance(mockedApi)); + const environments = mock(); + when(mockedApi.environments).thenReturn(instance(environments)); + when(environments.known).thenReturn([]); + setPythonApi(instance(mockedApi)); + }); + + teardown(() => { + setPythonApi(undefined as never); + sinon.restore(); + }); + + test('does not prompt when the toolkit is already installed', async () => { + when(installer.isInstalled(Product.deepnoteToolkit, anything())).thenResolve(true); + + const result = await service.ensureToolkitInstalled(interpreter, resource, notCancelled as never); + + assert.strictEqual(result, DeepnoteToolkitDependencyResponse.ok); + verify( + mockedVSCodeNamespaces.window.showInformationMessage(anything(), anything(), anything(), anything()) + ).never(); + verify(installer.install(anything(), anything(), anything())).never(); + }); + + test('installs only after the user consents', async () => { + when(installer.isInstalled(Product.deepnoteToolkit, anything())).thenResolve(false); + when(installer.install(anything(), anything(), anything())).thenResolve(InstallerResponse.Installed); + answerPrompt('Install'); + + const result = await service.ensureToolkitInstalled(interpreter, resource, notCancelled as never); + + assert.strictEqual(result, DeepnoteToolkitDependencyResponse.ok); + verify(installer.install(Product.deepnoteToolkit, anything(), anything())).once(); + }); + + test('does NOT install when the user dismisses the prompt', async () => { + when(installer.isInstalled(Product.deepnoteToolkit, anything())).thenResolve(false); + answerPrompt(undefined); + + const result = await service.ensureToolkitInstalled(interpreter, resource, notCancelled as never); + + assert.strictEqual(result, DeepnoteToolkitDependencyResponse.cancel); + verify(installer.install(anything(), anything(), anything())).never(); + }); + + test('does NOT install when the user opts to change interpreter, and opens the picker', async () => { + when(installer.isInstalled(Product.deepnoteToolkit, anything())).thenResolve(false); + answerPrompt('Select a different Interpreter'); + + const result = await service.ensureToolkitInstalled(interpreter, resource, notCancelled as never); + + assert.strictEqual(result, DeepnoteToolkitDependencyResponse.selectDifferentInterpreter); + verify(installer.install(anything(), anything(), anything())).never(); + verify(mockedVSCodeNamespaces.commands.executeCommand('python.setInterpreter')).once(); + }); + + test('reports a cancelled install as cancel, not failure', async () => { + when(installer.isInstalled(Product.deepnoteToolkit, anything())).thenResolve(false); + when(installer.install(anything(), anything(), anything())).thenResolve(InstallerResponse.Cancelled); + answerPrompt('Install'); + + const result = await service.ensureToolkitInstalled(interpreter, resource, notCancelled as never); + + assert.strictEqual(result, DeepnoteToolkitDependencyResponse.cancel); + }); + + test('reports an install that did not take as failed', async () => { + when(installer.isInstalled(Product.deepnoteToolkit, anything())).thenResolve(false); + when(installer.install(anything(), anything(), anything())).thenResolve(InstallerResponse.Ignore); + answerPrompt('Install'); + + const result = await service.ensureToolkitInstalled(interpreter, resource, notCancelled as never); + + assert.strictEqual(result, DeepnoteToolkitDependencyResponse.failed); + }); +}); diff --git a/src/kernels/deepnote/types.ts b/src/kernels/deepnote/types.ts index 57b13f2e92..a5c9bc551e 100644 --- a/src/kernels/deepnote/types.ts +++ b/src/kernels/deepnote/types.ts @@ -148,6 +148,32 @@ export interface IDeepnoteToolkitInstaller { getVenvHash(deepnoteFileUri: vscode.Uri): string; } +export enum DeepnoteToolkitDependencyResponse { + /** The toolkit is present, or the user approved the install and it succeeded. */ + ok, + /** The user declined or cancelled. Not a failure — nothing should be reported as an error. */ + cancel, + /** The user chose to point the workspace at a different interpreter instead. */ + selectDifferentInterpreter, + /** The install ran and did not succeed. */ + failed +} + +export const IDeepnoteToolkitDependencyService = Symbol('IDeepnoteToolkitDependencyService'); +export interface IDeepnoteToolkitDependencyService { + /** + * Ensures deepnote-toolkit is available in the interpreter, prompting for consent first. + * @param interpreter The interpreter the kernel will run in + * @param resource The notebook the check is running for, used for logging + * @param token Cancellation token to cancel the check or the install + */ + ensureToolkitInstalled( + interpreter: PythonEnvironment, + resource: vscode.Uri | undefined, + token: vscode.CancellationToken + ): Promise; +} + export const IDeepnoteServerStarter = Symbol('IDeepnoteServerStarter'); export interface IDeepnoteServerStarter { /** diff --git a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts index b4ce53ad36..ed62c2cb0d 100644 --- a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts +++ b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts @@ -6,6 +6,8 @@ import * as fs from 'fs'; import { inject, injectable, named, optional } from 'inversify'; import { CancellationToken, + CancellationTokenSource, + NotebookController, NotebookControllerAffinity, NotebookDocument, NotebookEditor, @@ -14,16 +16,19 @@ import { commands, env, l10n, + notebooks, window, workspace } from 'vscode'; import { DEEPNOTE_NOTEBOOK_TYPE, DeepnoteKernelConnectionMetadata, + DeepnoteToolkitDependencyResponse, IDeepnoteKernelAutoSelector, IDeepnoteLspClientManager, IDeepnoteServerProvider, IDeepnoteServerStarter, + IDeepnoteToolkitDependencyService, IServerHandleRegistry } from '../../kernels/deepnote/types'; import { createJupyterConnectionInfo } from '../../kernels/jupyter/jupyterUtils'; @@ -66,6 +71,8 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, private readonly notebookControllers = new Map(); // Track interpreter ID for each notebook private readonly notebookInterpreterIds = new Map(); + // Offered at open so the notebook has something selectable before any server exists + private readonly placeholderControllers = new Map(); constructor( @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @@ -84,7 +91,9 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, @inject(IDeepnoteServerStarter) private readonly serverStarter: IDeepnoteServerStarter, @inject(IOutputChannel) @named(STANDARD_OUTPUT_CHANNEL) private readonly outputChannel: IOutputChannel, @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, - @inject(IServerHandleRegistry) private readonly serverHandleRegistry: IServerHandleRegistry + @inject(IServerHandleRegistry) private readonly serverHandleRegistry: IServerHandleRegistry, + @inject(IDeepnoteToolkitDependencyService) + private readonly toolkitDependencyService: IDeepnoteToolkitDependencyService ) {} public activate() { @@ -118,45 +127,13 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, logger.info(`Deepnote notebook opened: ${getDisplayPath(notebook.uri)}`); - // Always try to ensure kernel is selected (this will reuse existing controllers) - // Don't await - let it happen in background so notebook opens quickly - window - .withProgress( - { - location: ProgressLocation.Notification, - title: l10n.t('Auto-selecting Deepnote kernel... {0}', getDisplayPath(notebook.uri)), - cancellable: true - }, - async (progress, token) => { - try { - const result = await this.ensureKernelSelected(notebook, progress, token); - return result; - } catch (error) { - logger.error( - `Failed to auto-select Deepnote kernel for ${getDisplayPath(notebook.uri)}`, - error - ); - void this.handleKernelSelectionError(error, notebook); - return true; - } - } - ) - .then( - (result) => { - logger.info(`Auto-selecting Deepnote kernel for ${getDisplayPath(notebook.uri)} result: ${result}`); - if (!result) { - logger.warn( - `No active Python interpreter found for ${getDisplayPath( - notebook.uri - )}, kernel not selected` - ); - } - }, - (error) => { - logger.error(`Error auto-selecting Deepnote kernel for ${getDisplayPath(notebook.uri)}`, error); - void this.handleKernelSelectionError(error, notebook); - } - ); + // Like the Jupyter extension, opening a notebook only offers a controller. The toolkit + // check, its consent prompt and the server start all wait for the first execution. + try { + await this.selectPlaceholderController(notebook); + } catch (error) { + logger.error(`Failed to offer a Deepnote kernel for ${getDisplayPath(notebook.uri)}`, error); + } } private onControllerSelectionChanged(event: { @@ -186,6 +163,13 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, this.notebookInterpreterIds.delete(notebookKey); this.notebookControllers.delete(notebookKey); + const placeholder = this.placeholderControllers.get(notebookKey); + + if (placeholder) { + placeholder.dispose(); + this.placeholderControllers.delete(notebookKey); + } + logger.info(`Deepnote notebook closed, cleaned up: ${getDisplayPath(notebook.uri)}`); } @@ -236,6 +220,14 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, return; } + const dependency = await this.toolkitDependencyService.ensureToolkitInstalled(interpreter, notebook.uri, token); + + if (dependency !== DeepnoteToolkitDependencyResponse.ok) { + logger.info(`deepnote-toolkit unavailable, controller not rebuilt for ${getDisplayPath(notebook.uri)}`); + + return; + } + await this.ensureKernelSelectedWithInterpreter(notebook, interpreter, notebookKey, progress, token); // Setup succeeded. If it registered a new server handle (full setup path), drop the old one. @@ -539,6 +531,43 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, return true; } + // Consent before installing into the user's interpreter. Declining or cancelling is not a + // failure: execution simply does not proceed and no error UI is raised. + let dependency: DeepnoteToolkitDependencyResponse; + + try { + dependency = await this.toolkitDependencyService.ensureToolkitInstalled(interpreter, notebook.uri, token); + } catch (error) { + if (token.isCancellationRequested || isCancellationError(error as Error)) { + logger.info(`deepnote-toolkit install cancelled for ${getDisplayPath(notebook.uri)}`); + + return false; + } + + await this.handleKernelSelectionError(error, notebook); + + return false; + } + + if (dependency === DeepnoteToolkitDependencyResponse.failed) { + await this.handleKernelSelectionError( + new Error(l10n.t('Failed to install {0}.', 'deepnote-toolkit')), + notebook + ); + + return false; + } + + if (dependency !== DeepnoteToolkitDependencyResponse.ok) { + logger.info( + `deepnote-toolkit unavailable for ${getDisplayPath(notebook.uri)} (${ + DeepnoteToolkitDependencyResponse[dependency] + }), kernel not started` + ); + + return false; + } + try { await window.withProgress( { @@ -605,6 +634,96 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, ); } + /** + * Offer a controller for a notebook whose kernel has not been set up yet, and select it so the + * notebook has something runnable. Running a cell through it performs the real setup. + */ + private async selectPlaceholderController(notebook: NotebookDocument): Promise { + const placeholder = this.createPlaceholderController(notebook); + placeholder.updateNotebookAffinity(notebook, NotebookControllerAffinity.Preferred); + + const notebookEditor = await this.findNotebookEditor(notebook); + + if (!notebookEditor) { + logger.warn( + `Could not find NotebookEditor for ${getDisplayPath(notebook.uri)}, kernel may not be selected` + ); + + return; + } + + await commands.executeCommand('notebook.selectKernel', { + notebookEditor, + id: placeholder.id, + extension: JVSC_EXTENSION_ID + }); + } + + /** + * One placeholder per notebook. Its execute handler runs the toolkit check, the consent prompt + * and the server start; the cells themselves run on the real controller once it exists. + */ + private createPlaceholderController(notebook: NotebookDocument): NotebookController { + const notebookKey = getNotebookKey(notebook.uri); + const existing = this.placeholderControllers.get(notebookKey); + + if (existing) { + return existing; + } + + const controller = notebooks.createNotebookController( + `deepnote-placeholder-${notebookKey}`, + DEEPNOTE_NOTEBOOK_TYPE, + l10n.t('Deepnote Kernel') + ); + + controller.supportsExecutionOrder = true; + controller.supportedLanguages = ['python', 'sql', 'markdown', 'plaintext']; + + controller.executeHandler = async (cells, doc) => { + logger.info(`Placeholder execute handler for ${getDisplayPath(doc.uri)} with ${cells.length} cells`); + + if (!workspace.isTrusted) { + logger.info(`Workspace is not trusted, skipping kernel setup for ${getDisplayPath(doc.uri)}`); + + return; + } + + const cts = new CancellationTokenSource(); + const closeListener = workspace.onDidCloseNotebookDocument((closedDoc) => { + if (getNotebookKey(closedDoc.uri) === getNotebookKey(doc.uri)) { + logger.info(`Notebook closed during kernel setup, cancelling`); + cts.cancel(); + } + }); + + try { + const ready = await this.ensureEnvironmentConfiguredBeforeExecution(doc, cts.token); + + if (!ready) { + logger.info(`Kernel not set up for ${getDisplayPath(doc.uri)}, cells not executed`); + + return; + } + + void window.showInformationMessage(l10n.t('Kernel ready. Run the cells again to execute them.')); + } catch (error) { + if (isCancellationError(error)) { + logger.info(`Kernel setup cancelled for ${getDisplayPath(doc.uri)}`); + } else { + logger.error(`Error in placeholder execute handler`, error); + } + } finally { + closeListener.dispose(); + cts.dispose(); + } + }; + + this.placeholderControllers.set(notebookKey, controller); + + return controller; + } + /** * Find the NotebookEditor for a given NotebookDocument. * Required for properly selecting a kernel with the notebook.selectKernel command. diff --git a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts index defb320fdd..f8f2888e32 100644 --- a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts +++ b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.unit.test.ts @@ -3,6 +3,7 @@ import * as sinon from 'sinon'; import { anything, instance, mock, verify, when } from 'ts-mockito'; import { DeepnoteKernelAutoSelector } from './deepnoteKernelAutoSelector.node'; import { ServerHandleRegistry } from '../../kernels/deepnote/deepnoteServerHandleRegistry.node'; +import { DeepnoteToolkitDependencyResponse, IDeepnoteToolkitDependencyService } from '../../kernels/deepnote/types'; import { IDeepnoteLspClientManager, IDeepnoteServerProvider, @@ -46,6 +47,7 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { let mockOutputChannel: IOutputChannel; let mockInterpreterService: IInterpreterService; let registry: ServerHandleRegistry; + let mockToolkitDependencyService: IDeepnoteToolkitDependencyService; let mockProgress: { report(value: { message?: string; increment?: number }): void }; let mockCancellationToken: CancellationToken; @@ -74,6 +76,10 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { mockOutputChannel = mock(); mockInterpreterService = mock(); registry = new ServerHandleRegistry(); + mockToolkitDependencyService = mock(); + when(mockToolkitDependencyService.ensureToolkitInstalled(anything(), anything(), anything())).thenResolve( + DeepnoteToolkitDependencyResponse.ok + ); mockProgress = { report: sandbox.stub() }; mockCancellationToken = mock(); @@ -137,7 +143,8 @@ suite('DeepnoteKernelAutoSelector - rebuildController', () => { instance(mockServerStarter), instance(mockOutputChannel), instance(mockInterpreterService), - registry + registry, + instance(mockToolkitDependencyService) ); }); diff --git a/src/notebooks/serviceRegistry.node.ts b/src/notebooks/serviceRegistry.node.ts index 7beb5c8916..52d0e09230 100644 --- a/src/notebooks/serviceRegistry.node.ts +++ b/src/notebooks/serviceRegistry.node.ts @@ -78,6 +78,7 @@ import { IDeepnoteEnvironmentManager, IDeepnoteNotebookEnvironmentMapper, IDeepnoteLspClientManager, + IDeepnoteToolkitDependencyService, IServerHandleRegistry } from '../kernels/deepnote/types'; import { DeepnoteAgentSkillsManager } from '../kernels/deepnote/deepnoteAgentSkillsManager.node'; @@ -86,6 +87,7 @@ import { DeepnoteServerStarter } from '../kernels/deepnote/deepnoteServerStarter import { DeepnoteKernelAutoSelector } from './deepnote/deepnoteKernelAutoSelector.node'; import { DeepnoteServerProvider } from '../kernels/deepnote/deepnoteServerProvider.node'; import { ServerHandleRegistry } from '../kernels/deepnote/deepnoteServerHandleRegistry.node'; +import { DeepnoteToolkitDependencyService } from '../kernels/deepnote/deepnoteToolkitDependencyService.node'; import { DeepnoteLspClientManager } from '../kernels/deepnote/deepnoteLspClientManager.node'; import { DeepnoteInitNotebookRunner } from './deepnote/deepnoteInitNotebookRunner.node'; import { DeepnoteRequirementsHelper, IDeepnoteRequirementsHelper } from './deepnote/deepnoteRequirementsHelper.node'; @@ -264,6 +266,10 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea serviceManager.addSingleton(IDeepnoteServerProvider, DeepnoteServerProvider); serviceManager.addBinding(IDeepnoteServerProvider, IExtensionSyncActivationService); serviceManager.addSingleton(IServerHandleRegistry, ServerHandleRegistry); + serviceManager.addSingleton( + IDeepnoteToolkitDependencyService, + DeepnoteToolkitDependencyService + ); serviceManager.addSingleton(IDeepnoteKernelAutoSelector, DeepnoteKernelAutoSelector); serviceManager.addBinding(IDeepnoteKernelAutoSelector, IExtensionSyncActivationService); serviceManager.addSingleton(IDeepnoteLspClientManager, DeepnoteLspClientManager); diff --git a/src/platform/common/utils/localize.ts b/src/platform/common/utils/localize.ts index 30d282fda7..7bf68e84fe 100644 --- a/src/platform/common/utils/localize.ts +++ b/src/platform/common/utils/localize.ts @@ -466,6 +466,7 @@ export namespace DataScience { l10n.t('Failure during variable extraction: \r\n{0}', errorMessage); export const selectKernel = l10n.t('Change Kernel'); export const selectDifferentKernel = l10n.t('Select a different Kernel'); + export const selectDifferentPythonInterpreter = l10n.t('Select a different Interpreter'); export const kernelFilterPlaceholder = l10n.t('Choose the kernels that are available in the kernel picker.'); export const recommendedItemCategoryInQuickPick = l10n.t('Recommended'); export const selectedKernelCategoryInQuickPick = l10n.t('Selected'); diff --git a/test/e2e/helpers/modals.ts b/test/e2e/helpers/modals.ts index 993935171f..4a42ee4f04 100644 --- a/test/e2e/helpers/modals.ts +++ b/test/e2e/helpers/modals.ts @@ -6,7 +6,10 @@ import { WORKBENCH_TIMEOUT } from './constants'; * Confirms a `{modal:true}` dialog by clicking the button matching `label`, driving the raw * `.monaco-dialog-box` (ExTester's `ModalDialog` attaches unreliably); `messageIncludes` disambiguates. */ -export async function confirmModalDialog(label: string, options?: { messageIncludes?: string }): Promise { +export async function confirmModalDialog( + label: string, + options?: { messageIncludes?: string; onVisible?: () => Promise } +): Promise { const driver = VSBrowser.instance.driver; const messageIncludes = options?.messageIncludes; @@ -25,6 +28,9 @@ export async function confirmModalDialog(label: string, options?: { messageInclu `modal dialog${messageIncludes ? ` containing "${messageIncludes}"` : ''} did not appear` ); + // Runs while the dialog is still up — the only chance to capture or inspect it. + await options?.onVisible?.(); + const button = await driver.wait( async () => { const selector = '.monaco-dialog-box .dialog-buttons .monaco-button, .monaco-dialog-box .monaco-button'; diff --git a/test/e2e/suite/interpreterKernel.e2e.test.ts b/test/e2e/suite/interpreterKernel.e2e.test.ts index 156adb317b..067af857c6 100644 --- a/test/e2e/suite/interpreterKernel.e2e.test.ts +++ b/test/e2e/suite/interpreterKernel.e2e.test.ts @@ -1,25 +1,22 @@ /** - * End-to-end UI test for kernel setup WITHOUT Deepnote environments. + * End-to-end UI test for kernel setup WITHOUT Deepnote environments, following the Jupyter + * extension's mechanism for a missing Python dependency: + * 1. opening a `.deepnote` file only OFFERS a kernel — nothing is installed and no server starts + * 2. running a cell detects that deepnote-toolkit is missing and asks for consent (modal prompt) + * 3. on "Install" the toolkit goes into the workspace's *active interpreter*, not a managed venv + * 4. the server starts, the real controller binds, and a re-run executes the cell * - * This is the flow the extension uses now that environments are gone: the kernel is built from the - * workspace's *active Python interpreter*, and deepnote-toolkit is installed into that interpreter - * through the Python extension's installer infrastructure (`IInstaller`) — the same mechanism the - * Jupyter extension uses for its own missing dependencies: - * 1. open a one-notebook `.deepnote` file against a workspace whose active interpreter is a bare venv - * 2. the auto-selector picks that interpreter — no environment is created and no picker appears - * 3. deepnote-toolkit is missing, so an "Installing deepnote-toolkit" progress notification shows - * 4. once it installs, the server starts and the kernel controller is bound - * 5. run the cell and assert the rendered stdout + * The workspace's active interpreter is a bare venv this test creates, so the install path runs on + * every execution rather than only on a machine that happens to be missing the package. The cell + * prints `sys.prefix`, so the output proves the kernel ran inside that venv. * - * The cell prints `sys.prefix`, so the output proves the kernel really ran inside the venv this test - * created rather than in a Deepnote-managed environment. The suite also asserts the toolkit landed in - * that same venv, which is the load-bearing difference from the environment-based flow. + * Screenshots are captured at each step into `test/e2e/screenshots/interpreterKernel/` so the flow + * can be confirmed visually — in particular that the consent prompt is actually shown. * * Prerequisites: - * - The Python extension (`ms-python.python`) must be installed in the test instance - * (`npm run setup:e2e:deps`). + * - The Python extension (`ms-python.python`) must be installed in the test instance. * - `python3` must be on PATH and able to create a venv (CI installs `python3.12-venv`). - * - Network access: the toolkit is installed from PyPI on first kernel start, which is slow. + * - Network access: the toolkit is installed from PyPI, which is slow. */ import { expect } from 'chai'; @@ -33,20 +30,22 @@ import { KERNEL_CONNECT_TIMEOUT, SUITE_TIMEOUT, WORKBENCH_TIMEOUT, + clickRunAll, + confirmModalDialog, copyFixtureToTempDir, + createScreenshotter, + dismissAllNotifications, openFolderViaDialog, openWorkspaceFile, runOnceAndAwaitOutput, - waitForNotification, - waitForNotificationToClear + waitForNotification } from '../helpers'; const NOTEBOOK_FILE_NAME = 'interpreter-kernel.deepnote'; const EXPECTED_OUTPUT = 'interpreter-kernel-ok'; -// The install toast is observed opportunistically; the venv check below is the real gate, so a -// missed toast must not cost the suite a full kernel-connect timeout. -const TOOLKIT_NOTIFICATION_TIMEOUT = 90_000; +/** How long the notebook is watched to prove that merely opening it installs nothing. */ +const NO_INSTALL_OBSERVATION_MS = 15_000; /** Path to the interpreter inside a venv, for the platform the test is running on. */ function venvPython(venvDir: string): string { @@ -66,7 +65,7 @@ function isToolkitInstalled(python: string): boolean { } } -describe('Deepnote E2E — run on the active interpreter (no Deepnote environment)', function () { +describe('Deepnote E2E — consent, then install into the active interpreter', function () { this.timeout(SUITE_TIMEOUT); let cleanupTempDir: (() => void) | undefined; @@ -77,9 +76,6 @@ describe('Deepnote E2E — run on the active interpreter (no Deepnote environmen const { cleanup, tempDir } = copyFixtureToTempDir(NOTEBOOK_FILE_NAME); cleanupTempDir = cleanup; - // A throwaway venv is what makes this test deterministic: it is guaranteed not to have - // deepnote-toolkit, so the install path runs on every execution rather than only on a - // machine that happens to be missing the package. venvDir = path.join(tempDir, '.venv'); execFileSync('python3', ['-m', 'venv', venvDir], { stdio: 'inherit' }); interpreter = venvPython(venvDir); @@ -89,8 +85,8 @@ describe('Deepnote E2E — run on the active interpreter (no Deepnote environmen 'precondition: the fresh venv must not already provide deepnote-toolkit' ); - // Pin the workspace's interpreter so the auto-selector resolves this venv and not whatever - // the Python extension would otherwise discover on the machine. + // Pin the workspace's interpreter so the kernel resolves this venv and not whatever the + // Python extension would otherwise discover on the machine. const vscodeDir = path.join(tempDir, '.vscode'); fs.mkdirSync(vscodeDir, { recursive: true }); fs.writeFileSync( @@ -99,9 +95,6 @@ describe('Deepnote E2E — run on the active interpreter (no Deepnote environmen ); await VSBrowser.instance.waitForWorkbench(WORKBENCH_TIMEOUT); - - // Opening the folder reloads the window, so the notebook is opened in the test body — that - // keeps the install notification, which fires during the open, inside the assertions. await openFolderViaDialog(tempDir); await VSBrowser.instance.waitForWorkbench(WORKBENCH_TIMEOUT); }); @@ -121,32 +114,54 @@ describe('Deepnote E2E — run on the active interpreter (no Deepnote environmen } }); - it('installs deepnote-toolkit into the active interpreter, then runs the cell', async function () { - await openWorkspaceFile(NOTEBOOK_FILE_NAME); + it('installs nothing on open, asks before installing, then runs the cell', async function () { + const shot = createScreenshotter(this); + const driver = VSBrowser.instance.driver; - await VSBrowser.instance.driver.wait( + await openWorkspaceFile(NOTEBOOK_FILE_NAME); + await driver.wait( async () => (await new EditorView().getOpenEditorTitles()).some((t) => t.includes(NOTEBOOK_FILE_NAME)), WORKBENCH_TIMEOUT, 'Deepnote notebook editor did not open' ); - // Opening the notebook auto-selects the kernel, which finds the toolkit missing and installs - // it — no environment is created and the user is never asked to pick one. The progress toast - // is the visible half of that, but it is transient (and absent on a retry, where the toolkit - // is already installed), so it is observed best-effort; the durable assertions are below. - await waitForNotification(/Installing deepnote-toolkit/i, TOOLKIT_NOTIFICATION_TIMEOUT, false); + await shot('notebook-open'); - // The load-bearing gate: the install landed in the active interpreter. Polling the venv is - // race-free, unlike matching a toast that may already have gone. - await VSBrowser.instance.driver.wait( + // The load-bearing half of "detect on kernel start, not notebook open". Watching the install + // toast rather than only the venv is what makes this catch a regression: an open-time install + // announces itself within seconds, long before the package would actually land on disk. + const installStartedOnOpen = await waitForNotification( + /Installing deepnote_toolkit/i, + NO_INSTALL_OBSERVATION_MS, + false + ); + + expect(installStartedOnOpen, 'opening the notebook must not start an install').to.equal(undefined); + expect(isToolkitInstalled(interpreter)).to.equal( + false, + 'opening the notebook must not install anything into the interpreter' + ); + + // Running a cell is the user gesture that triggers detection, and the consent prompt. + await dismissAllNotifications().catch(() => undefined); + await clickRunAll(NOTEBOOK_FILE_NAME); + + await confirmModalDialog('Install', { + messageIncludes: 'deepnote-toolkit', + onVisible: async () => { + await shot('consent-prompt'); + } + }); + + await driver.wait( () => isToolkitInstalled(interpreter), KERNEL_CONNECT_TIMEOUT, 'deepnote-toolkit was never installed into the active interpreter' ); - // Waiting for the auto-select toast to be *gone* gates "Run All" on a bound kernel without - // depending on catching it while it is shown. - await waitForNotificationToClear(/Auto-selecting Deepnote kernel/i, KERNEL_CONNECT_TIMEOUT); + // The first run only sets the kernel up; the cells themselves run on the re-run. + await waitForNotification(/Run the cells again/i, KERNEL_CONNECT_TIMEOUT, true); + await shot('kernel-ready'); const renderedOutput = await runOnceAndAwaitOutput( NOTEBOOK_FILE_NAME, @@ -154,6 +169,8 @@ describe('Deepnote E2E — run on the active interpreter (no Deepnote environmen FIRST_RUN_OUTPUT_TIMEOUT ); + await shot('cell-output'); + expect(renderedOutput).to.contain(EXPECTED_OUTPUT); // The cell printed sys.prefix: the kernel must be the venv this test created, which is what From f24b20f90ce0e996197c71511e49e3ce20f1adc0 Mon Sep 17 00:00:00 2001 From: tomas Date: Tue, 25 Aug 2026 18:01:21 +0000 Subject: [PATCH 6/7] refactor(deepnote): name kernels after the environment, as Jupyter does The Deepnote case returned the .deepnote project title, so the kernel picker described the document rather than the runtime: every notebook in a project carried the same label, and nothing told the user which interpreter the kernel would use -- the one thing worth checking before consenting to an install. Use the same implementation as 'startUsingPythonInterpreter': getDisplayNameOrNameOfPythonKernelConnection, which yields " (Python )" -- e.g. ".venv (Python 3.12.13)" -- falling back to "Python " for an unrecognised environment and to the kernelspec name when there is no interpreter at all. The environmentName fallback below it was already unreachable (projectName always had a value, defaulting to 'Untitled Project'), so the interpreter path this PR started storing there was never displayed. projectName is dropped from the connection metadata: it existed only for this label, is not serialized by toJSON, and the project title is still shown on the editor tab and in the Deepnote status bar. environmentName and notebookName stay -- they participate in connection equality. Verified: 2769 unit tests passing; the naming test fails when the environment branch is removed; E2E green with the picker confirmed reading ".venv (Python 3.12.13)". Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019cqzx43mLQRvVfWbfEUUSx --- src/kernels/deepnote/types.ts | 4 -- src/kernels/helpers.ts | 22 ++++++---- src/kernels/helpers.unit.test.ts | 42 +++++++++++++++++++ .../deepnoteKernelAutoSelector.node.ts | 4 -- 4 files changed, 56 insertions(+), 16 deletions(-) diff --git a/src/kernels/deepnote/types.ts b/src/kernels/deepnote/types.ts index a5c9bc551e..e98b268775 100644 --- a/src/kernels/deepnote/types.ts +++ b/src/kernels/deepnote/types.ts @@ -31,7 +31,6 @@ export class DeepnoteKernelConnectionMetadata { public readonly serverProviderHandle: JupyterServerProviderHandle; public readonly serverInfo?: DeepnoteServerInfo; // Store server info for connection public readonly environmentName?: string; // Name of the Deepnote environment for display purposes - public readonly projectName?: string; // Name of the project for display purposes public readonly notebookName?: string; // Name of the notebook for display purposes private constructor(options: { @@ -43,7 +42,6 @@ export class DeepnoteKernelConnectionMetadata { serverProviderHandle: JupyterServerProviderHandle; serverInfo?: DeepnoteServerInfo; environmentName?: string; - projectName?: string; notebookName?: string; }) { this.interpreter = options.interpreter; @@ -54,7 +52,6 @@ export class DeepnoteKernelConnectionMetadata { this.serverProviderHandle = options.serverProviderHandle; this.serverInfo = options.serverInfo; this.environmentName = options.environmentName; - this.projectName = options.projectName; this.notebookName = options.notebookName; } @@ -67,7 +64,6 @@ export class DeepnoteKernelConnectionMetadata { serverProviderHandle: JupyterServerProviderHandle; serverInfo?: DeepnoteServerInfo; environmentName?: string; - projectName?: string; notebookName?: string; }) { return new DeepnoteKernelConnectionMetadata(options); diff --git a/src/kernels/helpers.ts b/src/kernels/helpers.ts index 11c88baa21..1cc307c804 100644 --- a/src/kernels/helpers.ts +++ b/src/kernels/helpers.ts @@ -302,16 +302,22 @@ export function getDisplayNameOrNameOfKernelConnection(kernelConnection: KernelC return `Python ${pythonVersion}`.trim(); } case 'startUsingDeepnoteKernel': { - // Display as "Project Title" - if (kernelConnection.projectName) { - return kernelConnection.projectName; + // Named after the environment the code runs in, exactly as 'startUsingPythonInterpreter' + // does: the kernel picker answers "which interpreter?", and the project title is already + // carried by the editor tab and the Deepnote status bar. + if (!kernelConnection.interpreter) { + return oldDisplayName; } - // For Deepnote kernels, use the environment name if available - if (kernelConnection.environmentName) { - return `Deepnote: ${kernelConnection.environmentName}`; + + if (getEnvironmentType(kernelConnection.interpreter) !== EnvironmentType.Unknown) { + return getDisplayNameOrNameOfPythonKernelConnection(kernelConnection.interpreter); } - // Fallback to kernelspec display name - return oldDisplayName; + + const deepnotePythonVersion = ( + getTelemetrySafeVersion(getCachedVersion(kernelConnection.interpreter)) || '' + ).trim(); + + return `Python ${deepnotePythonVersion}`.trim(); } } return oldDisplayName; diff --git a/src/kernels/helpers.unit.test.ts b/src/kernels/helpers.unit.test.ts index 0d6d163881..bccc4e6acf 100644 --- a/src/kernels/helpers.unit.test.ts +++ b/src/kernels/helpers.unit.test.ts @@ -6,6 +6,7 @@ import { assert } from 'chai'; import { when, instance, mock, anything } from 'ts-mockito'; import { Uri } from 'vscode'; import { getDisplayNameOrNameOfKernelConnection } from './helpers'; +import { DeepnoteKernelConnectionMetadata } from './deepnote/types'; import { IJupyterKernelSpec, LiveRemoteKernelConnectionMetadata, @@ -261,6 +262,47 @@ suite('Kernel Connection Helpers', () => { assert.strictEqual(name, 'kspecname (.env)'); }); }); + suite('Deepnote kernels', () => { + const venv = { uri: Uri.file('/work/.venv/bin/python'), id: '/work/.venv/bin/python' }; + + function deepnoteConnection(interpreter?: PythonEnvironment) { + return DeepnoteKernelConnectionMetadata.create({ + id: 'deepnote-notebook-1', + baseUrl: 'http://127.0.0.1:8888', + kernelSpec: { + argv: [], + display_name: 'deepnote-kernelspec-name', + name: 'python3', + executable: 'python', + language: 'python' + }, + serverProviderHandle: { extensionId: 'ext', id: 'deepnote-server', handle: 'handle' }, + interpreter + }); + } + + test('is named after the environment, not the Deepnote project', () => { + whenKnownEnvironments(environments).thenReturn([ + { + id: venv.id, + path: venv.uri.fsPath, + environment: { name: '.venv', type: 'VirtualEnvironment' }, + version: { major: 3, minor: 12, micro: 13, sysVersion: '3.12.13' } + } + ]); + + const name = getDisplayNameOrNameOfKernelConnection(deepnoteConnection(venv as PythonEnvironment)); + + assert.strictEqual(name, '.venv (Python 3.12.13)'); + }); + + test('falls back to the kernelspec name when there is no interpreter', () => { + const name = getDisplayNameOrNameOfKernelConnection(deepnoteConnection(undefined)); + + assert.strictEqual(name, 'deepnote-kernelspec-name'); + }); + }); + suite('Python kernels (started using kernelspec)', () => { test('Display name if language is python', () => { const name = getDisplayNameOrNameOfKernelConnection( diff --git a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts index ed62c2cb0d..7b15df9c21 100644 --- a/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts +++ b/src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts @@ -361,9 +361,6 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, // controller instead of creating a new one, avoiding the DISPOSED error. const controllerId = `deepnote-notebook-${notebookKey}`; - // Extract project and notebook titles from metadata for display - const projectTitle = notebook.metadata?.deepnoteProjectName || 'Untitled Project'; - const newConnectionMetadata = DeepnoteKernelConnectionMetadata.create({ interpreter, kernelSpec, @@ -373,7 +370,6 @@ export class DeepnoteKernelAutoSelector implements IDeepnoteKernelAutoSelector, serverProviderHandle, serverInfo, environmentName: getDisplayPath(interpreter.uri), - projectName: projectTitle, notebookName: notebookKey }); From 8cae65711260a63a22d3b149b4f30c8f6d6e9c3c Mon Sep 17 00:00:00 2001 From: tomas Date: Tue, 25 Aug 2026 19:22:47 +0000 Subject: [PATCH 7/7] fix(deepnote): describe kernels by their interpreter path, as Jupyter does getKernelDisplayPathFromKernelConnection had no case for 'startUsingDeepnoteKernel', so it fell into the non-Python branch and took the description straight from kernelSpec.executable. The toolkit server serves the stock ipykernel spec, whose executable is a bare "python", so the kernel picker showed "/python" -- a path that does not exist. Group the kind with the other kernelspec-backed Python kinds. The branch already resolves a bare "python" through the connection's interpreter, so the description becomes the environment folder, rendered workspace-relative -- ".venv" for a project-local venv, matching upstream. Verified: the description test fails against the old branch ("/python" instead of "/work/.venv"); 2770 unit tests passing; the E2E now opens the kernel picker and captures it, confirming the entry reads ".venv (Python 3.12.13)" with ".venv" as its description. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019cqzx43mLQRvVfWbfEUUSx --- src/kernels/helpers.ts | 3 ++- src/kernels/helpers.unit.test.ts | 18 +++++++++++++++++- test/e2e/suite/interpreterKernel.e2e.test.ts | 15 ++++++++++++++- 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/kernels/helpers.ts b/src/kernels/helpers.ts index 1cc307c804..938bad5880 100644 --- a/src/kernels/helpers.ts +++ b/src/kernels/helpers.ts @@ -381,7 +381,8 @@ export function getKernelDisplayPathFromKernelConnection(kernelConnection?: Kern if ( kernelConnection.kind === 'startUsingPythonInterpreter' || ((kernelConnection.kind === 'startUsingRemoteKernelSpec' || - kernelConnection.kind === 'startUsingLocalKernelSpec') && + kernelConnection.kind === 'startUsingLocalKernelSpec' || + kernelConnection.kind === 'startUsingDeepnoteKernel') && kernelConnection.kernelSpec.language === PYTHON_LANGUAGE) ) { const pathValue = diff --git a/src/kernels/helpers.unit.test.ts b/src/kernels/helpers.unit.test.ts index bccc4e6acf..49f410b37f 100644 --- a/src/kernels/helpers.unit.test.ts +++ b/src/kernels/helpers.unit.test.ts @@ -5,7 +5,7 @@ import * as sinon from 'sinon'; import { assert } from 'chai'; import { when, instance, mock, anything } from 'ts-mockito'; import { Uri } from 'vscode'; -import { getDisplayNameOrNameOfKernelConnection } from './helpers'; +import { getDisplayNameOrNameOfKernelConnection, getKernelDisplayPathFromKernelConnection } from './helpers'; import { DeepnoteKernelConnectionMetadata } from './deepnote/types'; import { IJupyterKernelSpec, @@ -296,6 +296,22 @@ suite('Kernel Connection Helpers', () => { assert.strictEqual(name, '.venv (Python 3.12.13)'); }); + test('describes the interpreter, resolving a relative kernelspec executable via the environment', () => { + whenKnownEnvironments(environments).thenReturn([ + { + id: venv.id, + path: venv.uri.fsPath, + environment: { name: '.venv', type: 'VirtualEnvironment', folderUri: Uri.file('/work/.venv') }, + version: { major: 3, minor: 12, micro: 13, sysVersion: '3.12.13' } + } + ]); + + // The toolkit server serves the stock ipykernel spec, whose executable is a bare "python". + const displayPath = getKernelDisplayPathFromKernelConnection(deepnoteConnection(venv as PythonEnvironment)); + + assert.strictEqual(displayPath?.fsPath, Uri.file('/work/.venv').fsPath); + }); + test('falls back to the kernelspec name when there is no interpreter', () => { const name = getDisplayNameOrNameOfKernelConnection(deepnoteConnection(undefined)); diff --git a/test/e2e/suite/interpreterKernel.e2e.test.ts b/test/e2e/suite/interpreterKernel.e2e.test.ts index 067af857c6..d80bcd3309 100644 --- a/test/e2e/suite/interpreterKernel.e2e.test.ts +++ b/test/e2e/suite/interpreterKernel.e2e.test.ts @@ -23,11 +23,12 @@ import { expect } from 'chai'; import { execFileSync } from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; -import { EditorView, VSBrowser, WebView } from 'vscode-extension-tester'; +import { EditorView, VSBrowser, WebView, Workbench } from 'vscode-extension-tester'; import { FIRST_RUN_OUTPUT_TIMEOUT, KERNEL_CONNECT_TIMEOUT, + QUICK_PICK_TIMEOUT, SUITE_TIMEOUT, WORKBENCH_TIMEOUT, clickRunAll, @@ -38,6 +39,7 @@ import { openFolderViaDialog, openWorkspaceFile, runOnceAndAwaitOutput, + tryOpenInputBox, waitForNotification } from '../helpers'; @@ -176,5 +178,16 @@ describe('Deepnote E2E — consent, then install into the active interpreter', f // The cell printed sys.prefix: the kernel must be the venv this test created, which is what // separates "active interpreter" from the old Deepnote-managed environment. expect(renderedOutput).to.contain(venvDir); + + // The kernel picker is the only place the description is rendered, so open it to capture + // both halves of the entry: the environment name as the label, its path as the description. + await new Workbench().executeCommand('notebook.selectKernel'); + + const picker = await tryOpenInputBox(QUICK_PICK_TIMEOUT); + + expect(picker, 'the kernel picker should open').to.not.equal(undefined); + + await shot('kernel-picker'); + await picker?.cancel(); }); });