diff --git a/.gitignore b/.gitignore index 161357f41..819f78389 100644 --- a/.gitignore +++ b/.gitignore @@ -3,7 +3,7 @@ dist node_modules /*.vsix .claude -.tasks +.plans .vscode-test/ .DS_Store e2e-testing/.resources diff --git a/.vscode/settings.json b/.vscode/settings.json index cebd853da..6b687d9c3 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -10,6 +10,10 @@ "out": true // set this to false to include "out" folder in search results }, // Turn off tsc task auto detection since we have the necessary tasks as npm scripts - "typescript.tsc.autoDetect": "off", - "typescript.tsdk": "node_modules/typescript/lib" + "js/ts.tsc.autoDetect": "off", + // Unit tests are excluded from the primary tsconfig.json used by VS Code + // editor. Setting the implicit project target to match the tsconfig.json so + // that the editor shows proper language features for the test files. + "js/ts.implicitProjectConfig.target": "ES2023", + "js/ts.tsdk.path": "node_modules/typescript/lib" } diff --git a/e2e-testing/src/specs/statusBar.spec.ts b/e2e-testing/src/specs/statusBar.spec.ts index 8359892b1..89fafa794 100644 --- a/e2e-testing/src/specs/statusBar.spec.ts +++ b/e2e-testing/src/specs/statusBar.spec.ts @@ -90,7 +90,7 @@ describe('Status Bar Tests', () => { await step(4, 'Verify connection node', async stepLabel => { const simpleTickingEditor = await getSidebarViewItem( - VIEW_NAME.connections, + VIEW_NAME.workers, SIMPLE_TICKING3_PY.name ); diff --git a/e2e-testing/src/util/constants.ts b/e2e-testing/src/util/constants.ts index 53af58fd4..1c06f3960 100644 --- a/e2e-testing/src/util/constants.ts +++ b/e2e-testing/src/util/constants.ts @@ -15,6 +15,6 @@ export const RETRY_SWITCH_IFRAME_ERRORS: ReadonlySet = new Set([ export const VIEW_NAME = { servers: 'Servers', - connections: 'Connections', + workers: 'Workers', panels: 'Panels', } as const; diff --git a/package-lock.json b/package-lock.json index bc4679aea..a013b203e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,8 +11,8 @@ "packages/*" ], "dependencies": { - "@deephaven-enterprise/auth-nodejs": "^1.20250219.134-beta", - "@deephaven-enterprise/query-utils": "^1.20250219.134-beta", + "@deephaven-enterprise/auth-nodejs": "^2026.1.38", + "@deephaven-enterprise/query-utils": "^2026.1.38", "@deephaven/jsapi-nodejs": "^1.15.0", "@deephaven/jsapi-utils": "^1.16.0", "@modelcontextprotocol/sdk": "^1.27.1", @@ -23,7 +23,7 @@ }, "devDependencies": { "@cfworker/json-schema": "^4.1.1", - "@deephaven-enterprise/jsapi-types": "^1.20250219.134-beta", + "@deephaven-enterprise/jsapi-types": "^2026.1.38", "@deephaven/jsapi-types": "^41.2.0", "@react-types/shared": "^3.33.1", "@types/archiver": "^6.0.3", @@ -556,9 +556,9 @@ } }, "node_modules/@deephaven-enterprise/auth-nodejs": { - "version": "1.20250219.134-beta", - "resolved": "https://registry.npmjs.org/@deephaven-enterprise/auth-nodejs/-/auth-nodejs-1.20250219.134-beta.tgz", - "integrity": "sha512-INkEml6Al6dQckHhx2gKy53uk2QMb+1IcpL0mu5Z7H7kfRcH7o54KfB9j8DsSYmFZyjdRd1wmUzgfAf8xtxqQA==", + "version": "2026.1.38", + "resolved": "https://registry.npmjs.org/@deephaven-enterprise/auth-nodejs/-/auth-nodejs-2026.1.38.tgz", + "integrity": "sha512-eMSlC52lQSnuPIiarHKBv7Yu5lufgKYgHXlrFWxAR+1UJIGf7SgtiW44X6ijsY1bSPrJAjgbaLUnD3j6DtlX2g==", "license": "SEE LICENSE IN LICENSE.md", "dependencies": { "@deephaven-enterprise/jsapi-types": "file:../jsapi-types", @@ -570,9 +570,9 @@ "link": true }, "node_modules/@deephaven-enterprise/jsapi-types": { - "version": "1.20250219.134-beta", - "resolved": "https://registry.npmjs.org/@deephaven-enterprise/jsapi-types/-/jsapi-types-1.20250219.134-beta.tgz", - "integrity": "sha512-PRb6Z2J4msFDgooPTcGinml4xcoTc/YYkxK5PVOYV8ax71JXeicsGl4ndIcmgDZwdaciJhvVwhtuY5lMJkmTMg==", + "version": "2026.1.38", + "resolved": "https://registry.npmjs.org/@deephaven-enterprise/jsapi-types/-/jsapi-types-2026.1.38.tgz", + "integrity": "sha512-k+VEeI0QSF3eCqpc+N3LDNiC1WBfJ7yVEz1eLaFzHerYY7JwRKmJe6WmPjFn6CuMozqE/yhKAor3fFyPu5NltQ==", "license": "SEE LICENSE IN LICENSE.md", "dependencies": { "@deephaven/jsapi-types": "^1.0.0-dev0.36.1" @@ -585,11 +585,12 @@ "license": "Apache-2.0" }, "node_modules/@deephaven-enterprise/query-utils": { - "version": "1.20250219.134-beta", - "resolved": "https://registry.npmjs.org/@deephaven-enterprise/query-utils/-/query-utils-1.20250219.134-beta.tgz", - "integrity": "sha512-zkrzFGRgWScDvMA1/tLBE/LtwFDKEoZYSiOdxtgih2+xzP6mgn+VnjWevtcBxPTDJCf6+n7+R+YfGW7TqJtXcw==", + "version": "2026.1.38", + "resolved": "https://registry.npmjs.org/@deephaven-enterprise/query-utils/-/query-utils-2026.1.38.tgz", + "integrity": "sha512-9BOb/0mJP/kst72aL4jmsRmidR+Z9gXkihxnfILdWLU/D/Go+KRZ5ObYVHPiO2ntiJ1ymYxUFie6TMNpb31WyQ==", "license": "SEE LICENSE IN LICENSE.md", "dependencies": { + "@deephaven-enterprise/jsapi-types": "file:../jsapi-types", "@deephaven/jsapi-types": "^1.0.0-dev0.36.1", "@deephaven/log": "^0.97.0", "@deephaven/utils": "^0.97.0", @@ -597,6 +598,10 @@ "nanoid": "^5.1.6" } }, + "node_modules/@deephaven-enterprise/query-utils/node_modules/@deephaven-enterprise/jsapi-types": { + "resolved": "node_modules/@deephaven-enterprise/jsapi-types", + "link": true + }, "node_modules/@deephaven-enterprise/query-utils/node_modules/@deephaven/jsapi-types": { "version": "1.0.0-dev0.40.9", "resolved": "https://registry.npmjs.org/@deephaven/jsapi-types/-/jsapi-types-1.0.0-dev0.40.9.tgz", @@ -15000,9 +15005,9 @@ "dev": true }, "@deephaven-enterprise/auth-nodejs": { - "version": "1.20250219.134-beta", - "resolved": "https://registry.npmjs.org/@deephaven-enterprise/auth-nodejs/-/auth-nodejs-1.20250219.134-beta.tgz", - "integrity": "sha512-INkEml6Al6dQckHhx2gKy53uk2QMb+1IcpL0mu5Z7H7kfRcH7o54KfB9j8DsSYmFZyjdRd1wmUzgfAf8xtxqQA==", + "version": "2026.1.38", + "resolved": "https://registry.npmjs.org/@deephaven-enterprise/auth-nodejs/-/auth-nodejs-2026.1.38.tgz", + "integrity": "sha512-eMSlC52lQSnuPIiarHKBv7Yu5lufgKYgHXlrFWxAR+1UJIGf7SgtiW44X6ijsY1bSPrJAjgbaLUnD3j6DtlX2g==", "requires": { "@deephaven-enterprise/jsapi-types": "file:../jsapi-types", "@deephaven/utils": "^0.97.0" @@ -15024,9 +15029,9 @@ } }, "@deephaven-enterprise/jsapi-types": { - "version": "1.20250219.134-beta", - "resolved": "https://registry.npmjs.org/@deephaven-enterprise/jsapi-types/-/jsapi-types-1.20250219.134-beta.tgz", - "integrity": "sha512-PRb6Z2J4msFDgooPTcGinml4xcoTc/YYkxK5PVOYV8ax71JXeicsGl4ndIcmgDZwdaciJhvVwhtuY5lMJkmTMg==", + "version": "2026.1.38", + "resolved": "https://registry.npmjs.org/@deephaven-enterprise/jsapi-types/-/jsapi-types-2026.1.38.tgz", + "integrity": "sha512-k+VEeI0QSF3eCqpc+N3LDNiC1WBfJ7yVEz1eLaFzHerYY7JwRKmJe6WmPjFn6CuMozqE/yhKAor3fFyPu5NltQ==", "requires": { "@deephaven/jsapi-types": "^1.0.0-dev0.36.1" }, @@ -15039,10 +15044,11 @@ } }, "@deephaven-enterprise/query-utils": { - "version": "1.20250219.134-beta", - "resolved": "https://registry.npmjs.org/@deephaven-enterprise/query-utils/-/query-utils-1.20250219.134-beta.tgz", - "integrity": "sha512-zkrzFGRgWScDvMA1/tLBE/LtwFDKEoZYSiOdxtgih2+xzP6mgn+VnjWevtcBxPTDJCf6+n7+R+YfGW7TqJtXcw==", + "version": "2026.1.38", + "resolved": "https://registry.npmjs.org/@deephaven-enterprise/query-utils/-/query-utils-2026.1.38.tgz", + "integrity": "sha512-9BOb/0mJP/kst72aL4jmsRmidR+Z9gXkihxnfILdWLU/D/Go+KRZ5ObYVHPiO2ntiJ1ymYxUFie6TMNpb31WyQ==", "requires": { + "@deephaven-enterprise/jsapi-types": "file:../jsapi-types", "@deephaven/jsapi-types": "^1.0.0-dev0.36.1", "@deephaven/log": "^0.97.0", "@deephaven/utils": "^0.97.0", @@ -15050,6 +15056,19 @@ "nanoid": "^5.1.6" }, "dependencies": { + "@deephaven-enterprise/jsapi-types": { + "version": "file:node_modules/@deephaven-enterprise/jsapi-types", + "requires": { + "@deephaven/jsapi-types": "^1.0.0-dev0.36.1" + }, + "dependencies": { + "@deephaven/jsapi-types": { + "version": "1.0.0-dev0.40.9", + "resolved": "https://registry.npmjs.org/@deephaven/jsapi-types/-/jsapi-types-1.0.0-dev0.40.9.tgz", + "integrity": "sha512-NwMxFmNCnRV4/2+MdN/8vUGiEtXFgL1K/+iXTKKvi+Brje5JHOSCn2miCKR9tAn0LNb/UdmJq+DSIZqvz8cU/Q==" + } + } + }, "@deephaven/jsapi-types": { "version": "1.0.0-dev0.40.9", "resolved": "https://registry.npmjs.org/@deephaven/jsapi-types/-/jsapi-types-1.0.0-dev0.40.9.tgz", diff --git a/package.json b/package.json index 1e7374e9b..7ff79d550 100644 --- a/package.json +++ b/package.json @@ -36,6 +36,7 @@ "docs:start": "./scripts/startDocs", "docs:format": "./scripts/formatDocs", "docs:validate": "./scripts/validateDocs", + "format": "prettier --write \"src/**/*.{ts,js}\"", "icon:gen:dh": "node icons/generate.mjs dh", "icon:gen:ext": "node icons/generate.mjs dh-ext icons/src", "package:dev": "./scripts/package-dev.sh", @@ -244,6 +245,11 @@ "title": "Connect to Server as Another User", "icon": "$(account)" }, + { + "command": "vscode-deephaven.createWorker", + "title": "Create Worker", + "icon": "$(add)" + }, { "command": "vscode-deephaven.createNewTextDoc", "title": "New File", @@ -259,6 +265,11 @@ "title": "Disconnect from Server", "icon": "$(debug-disconnect)" }, + { + "command": "vscode-deephaven.disconnectFromWorker", + "title": "Disconnect from Worker", + "icon": "$(debug-disconnect)" + }, { "command": "vscode-deephaven.generateDHEKeyPair", "title": "Generate DHE Key Pair", @@ -834,6 +845,10 @@ "command": "vscode-deephaven.connectToServerOperateAs", "when": "false" }, + { + "command": "vscode-deephaven.createWorker", + "when": "false" + }, { "command": "vscode-deephaven.disconnectEditor", "when": "false" @@ -842,6 +857,10 @@ "command": "vscode-deephaven.disconnectFromServer", "when": "false" }, + { + "command": "vscode-deephaven.disconnectFromWorker", + "when": "false" + }, { "command": "vscode-deephaven.generateDHEKeyPair", "when:": "false" @@ -1005,9 +1024,14 @@ }, { "command": "vscode-deephaven.createNewTextDoc", - "when": "view == vscode-deephaven.view.serverConnectionTree && viewItem == isConnectionConnected", + "when": "view == vscode-deephaven.view.serverConnectionTree && (viewItem == isConnectionConnected || viewItem == isConnectionConnectedRemovable)", "group": "inline@1" }, + { + "command": "vscode-deephaven.createWorker", + "when": "view == vscode-deephaven.view.serverConnectionTree && viewItem == isDHEServerConnectionParent", + "group": "inline" + }, { "command": "vscode-deephaven.disconnectEditor", "when": "view == vscode-deephaven.view.serverConnectionTree && viewItem == isUri", @@ -1015,7 +1039,12 @@ }, { "command": "vscode-deephaven.disconnectFromServer", - "when": "(view == vscode-deephaven.view.serverTree && (viewItem == isServerRunningConnected || viewItem == isDHEServerRunningConnected)) || (view == vscode-deephaven.view.serverConnectionTree && (viewItem == isConnectionConnected || viewItem == isConnectionConnecting))", + "when": "view == vscode-deephaven.view.serverTree && (viewItem == isServerRunningConnected || viewItem == isDHEServerRunningConnected)", + "group": "inline@2" + }, + { + "command": "vscode-deephaven.disconnectFromWorker", + "when": "view == vscode-deephaven.view.serverConnectionTree && (viewItem == isConnectionConnectedRemovable || viewItem == isConnectionConnectingRemovable)", "group": "inline@2" }, { @@ -1024,7 +1053,7 @@ }, { "command": "vscode-deephaven.generateRequirementsTxt", - "when": "view == vscode-deephaven.view.serverConnectionTree && viewItem == isConnectionConnected" + "when": "view == vscode-deephaven.view.serverConnectionTree && (viewItem == isConnectionConnected || viewItem == isConnectionConnectedRemovable)" }, { "command": "vscode-deephaven.openInBrowser", @@ -1096,7 +1125,7 @@ }, { "id": "vscode-deephaven.view.serverConnectionTree", - "name": "Connections", + "name": "Workers", "type": "tree", "icon": "images/dh-logo-28.svg" }, @@ -1125,8 +1154,8 @@ } }, "dependencies": { - "@deephaven-enterprise/auth-nodejs": "^1.20250219.134-beta", - "@deephaven-enterprise/query-utils": "^1.20250219.134-beta", + "@deephaven-enterprise/auth-nodejs": "^2026.1.38", + "@deephaven-enterprise/query-utils": "^2026.1.38", "@deephaven/jsapi-nodejs": "^1.15.0", "@deephaven/jsapi-utils": "^1.16.0", "@modelcontextprotocol/sdk": "^1.27.1", @@ -1137,7 +1166,7 @@ }, "devDependencies": { "@cfworker/json-schema": "^4.1.1", - "@deephaven-enterprise/jsapi-types": "^1.20250219.134-beta", + "@deephaven-enterprise/jsapi-types": "^2026.1.38", "@deephaven/jsapi-types": "^41.2.0", "@react-types/shared": "^3.33.1", "@types/archiver": "^6.0.3", diff --git a/src/common/commands.ts b/src/common/commands.ts index ee2042f83..3f78a953e 100644 --- a/src/common/commands.ts +++ b/src/common/commands.ts @@ -26,6 +26,11 @@ export type ConnectToServerCmdArgs = [ operateAsAnotherUser?: boolean, ]; +/** Arguments passed to `CREATE_WORKER_CMD` handler */ +export type CreateWorkerCmdArgs = [ + serverState: Pick, +]; + /** Arguments passed to `OPEN_VARIABLE_PANELS_CMD` handler */ export type OpenVariablePanelsCmdArgs = [ serverUrl: URL, @@ -97,9 +102,11 @@ export const CREATE_DHE_AUTHENTICATED_CLIENT_CMD = cmd( 'createDHEAuthenticatedClient' ); export const CREATE_NEW_TEXT_DOC_CMD = cmd('createNewTextDoc'); +export const CREATE_WORKER_CMD = cmd('createWorker'); export const DELETE_VARIABLE_CMD = cmd('deleteVariable'); export const DISCONNECT_EDITOR_CMD = cmd('disconnectEditor'); export const DISCONNECT_FROM_SERVER_CMD = cmd('disconnectFromServer'); +export const DISCONNECT_FROM_WORKER_CMD = cmd('disconnectFromWorker'); export const DOWNLOAD_LOGS_CMD = cmd('downloadLogs'); export const GENERATE_DHE_KEY_PAIR_CMD = cmd('generateDHEKeyPair'); export const GENERATE_REQUIREMENTS_TXT_CMD = cmd('generateRequirementsTxt'); diff --git a/src/common/constants.ts b/src/common/constants.ts index 3506aa0ce..90a597b66 100644 --- a/src/common/constants.ts +++ b/src/common/constants.ts @@ -157,8 +157,11 @@ export const VARIABLE_UNICODE_ICONS = { /* eslint-enable @typescript-eslint/naming-convention */ export const CONNECTION_TREE_ITEM_CONTEXT = { - isConnectionConnected: 'isConnectionConnected', - isConnectionConnecting: 'isConnectionConnecting', + isConnectionConnected: (isOwned: boolean) => + `isConnectionConnected${isOwned ? 'Removable' : ''}`, + isConnectionConnecting: (isOwned: boolean) => + `isConnectionConnecting${isOwned ? 'Removable' : ''}`, + isDHEServerConnectionParent: 'isDHEServerConnectionParent', isUri: 'isUri', } as const; @@ -171,6 +174,7 @@ export const SERVER_TREE_ITEM_CONTEXT = { isManagedServerConnected: 'isManagedServerConnected', isManagedServerConnecting: 'isManagedServerConnecting', isManagedServerDisconnected: 'isManagedServerDisconnected', + isServerConnecting: 'isServerConnecting', isServerRunningConnected: 'isServerRunningConnected', isServerRunningDisconnected: 'isServerRunningDisconnected', isServerStopped: 'isServerStopped', diff --git a/src/controllers/ConnectionController.ts b/src/controllers/ConnectionController.ts index bef0cfaff..514fcc221 100644 --- a/src/controllers/ConnectionController.ts +++ b/src/controllers/ConnectionController.ts @@ -24,8 +24,11 @@ import { CONNECT_TO_SERVER_CMD, CONNECT_TO_SERVER_OPERATE_AS_CMD, ConnectToServerCmdArgs, + CREATE_WORKER_CMD, + CreateWorkerCmdArgs, DISCONNECT_EDITOR_CMD, DISCONNECT_FROM_SERVER_CMD, + DISCONNECT_FROM_WORKER_CMD, SELECT_CONNECTION_COMMAND, UnsupportedConsoleTypeError, } from '../common'; @@ -66,6 +69,9 @@ export class ConnectionController this.onConnectToServerOperateAs ); + /** Create a new worker on a DHE server */ + this.registerCommand(CREATE_WORKER_CMD, this.onCreateWorker); + /** Disconnect editor */ this.registerCommand(DISCONNECT_EDITOR_CMD, this.onDisconnectEditor); @@ -75,6 +81,12 @@ export class ConnectionController this.onDisconnectFromServer ); + /** Disconnect from worker (per-worker action on connection nodes) */ + this.registerCommand( + DISCONNECT_FROM_WORKER_CMD, + this.onDisconnectFromServer + ); + /** Select connection to run scripts against */ this.registerCommand( SELECT_CONNECTION_COMMAND, @@ -185,6 +197,10 @@ export class ConnectionController ); if (cn == null) { + updateConnectionStatusBarItem( + this._connectStatusBarItem, + 'disconnected' + ); return; } @@ -334,6 +350,24 @@ export class ConnectionController ); }; + /** + * Handle explicitly creating a new worker on a DHE server (the "+" action). + */ + onCreateWorker = async ( + ...[serverState]: CreateWorkerCmdArgs | [undefined] + ): Promise => { + // Sometimes view/item/context commands pass undefined instead of a value. + // Just ignore. microsoft/vscode#283655 + if (serverState == null) { + return; + } + + const languageId = vscode.window.activeTextEditor?.document.languageId; + const workerConsoleType = getConsoleType(languageId); + + await this._serverManager?.createWorker(serverState.url, workerConsoleType); + }; + /** * Handle connecting to a server as another user. * @param serverState @@ -473,10 +507,11 @@ export class ConnectionController try { selectedCnResult = await createConnectionQuickPick( - createConnectionQuickPickOptions( + await createConnectionQuickPickOptions( [...runningDHCServersWithoutConnections, ...runningDHEServers], connectionsForConsoleType, languageId, + this._serverManager, editorActiveConnectionUrl ) ); diff --git a/src/controllers/ServerConnectionTreeDragAndDropController.ts b/src/controllers/ServerConnectionTreeDragAndDropController.ts index 9ed518904..6ff4f5704 100644 --- a/src/controllers/ServerConnectionTreeDragAndDropController.ts +++ b/src/controllers/ServerConnectionTreeDragAndDropController.ts @@ -1,5 +1,5 @@ import { MIME_TYPE } from '../common'; -import { getEditorForUri } from '../util'; +import { getEditorForUri, isServerStateNode } from '../util'; import type { IServerManager } from '../types'; import type { ServerConnectionNode } from '../types/treeViewTypes'; @@ -21,8 +21,13 @@ export class ServerConnectionTreeDragAndDropController dataTransfer: vscode.DataTransfer, _token: vscode.CancellationToken ): Promise => { - // Only target connection nodes - if (target == null || target instanceof vscode.Uri) { + // Only target connection nodes. DHE server group nodes and uri leaf nodes + // are not valid editor-connection targets. + if ( + target == null || + target instanceof vscode.Uri || + isServerStateNode(target) + ) { return; } diff --git a/src/dh/ambientModules.d.ts b/src/dh/ambientModules.d.ts new file mode 100644 index 000000000..490547c8f --- /dev/null +++ b/src/dh/ambientModules.d.ts @@ -0,0 +1,19 @@ +/** + * Ambient module declarations for packages that are referenced by the type + * declarations of our dependencies but are not installed in this project. + * + * IMPORTANT: This file must stay a global *script* — do NOT add any top-level + * `import` or `export`. A `.d.ts` with a top-level import/export becomes a + * *module*, and inside a module `declare module 'x'` is treated as + * *augmentation* of an already-existing module. These modules are not + * installed, so there is nothing to augment; only a script-file *ambient module + * declaration* actually creates the module and lets the import resolve. + */ + +// `@deephaven-enterprise/query-utils@2026.x` ships a `.d.ts` (QueryUtils.d.ts) +// that does `import type { UriVariableDescriptor } from '@deephaven/jsapi-bootstrap'`, +// but does not declare `@deephaven/jsapi-bootstrap` as a dependency and it is +// not installed. Declare a minimal stub so the type import resolves (TS2307). +declare module '@deephaven/jsapi-bootstrap' { + type UriVariableDescriptor = string; +} diff --git a/src/dh/dhe.ts b/src/dh/dhe.ts index 82c828e45..3f683e929 100644 --- a/src/dh/dhe.ts +++ b/src/dh/dhe.ts @@ -42,7 +42,7 @@ import { UnsupportedFeatureQueryError, } from '../common'; import { withResolvers } from '../util'; -import type { QuerySerial } from '../shared'; +import { assertDefined, type QuerySerial } from '../shared'; export type IDraftQuery = EditableQueryInfo & { isClientSide: boolean; @@ -220,7 +220,7 @@ export async function loginClientWrapper( * @param consoleType The type of console to create. * @returns A promise that resolves to the serial of the created query. Note * that this will resolve before the query is actually ready to use. Use - * `getWorkerInfoFromQuery` to get the worker info when the query is ready. + * `getWorkerInfoFromQuerySerial` to get the worker info when the query is ready. */ export async function createInteractiveConsoleQuery( tagId: UniqueID, @@ -411,6 +411,89 @@ export async function getDheAuthConfig( return authConfig; } +/** + * Determine if a given query info represents an attachable IC worker for the + * current effective user. An attachable worker is an InteractiveConsole type, + * owned by `operateAs`, and currently Running. + * @param queryInfo Query info to check. + * @param operateAs The effective user to match ownership against. + * @returns True if the query is attachable, false otherwise. + */ +export function isAttachableWorker( + queryInfo: QueryInfo, + operateAs: string | null +): boolean { + return ( + queryInfo.type === INTERACTIVE_CONSOLE_QUERY_TYPE && + queryInfo.owner === operateAs && + queryInfo.designated?.status === 'Running' + ); +} + +/** + * List all running InteractiveConsole workers owned by the current effective + * user. + * @param dheClient DHE client to use. + * @param exclude Iterable of query serials to exclude from the results. + * @returns A promise resolving to the filtered QueryInfo array. + */ +export async function listAttachableWorkers( + dheClient: DheAuthenticatedClient, + exclude: Iterable +): Promise { + const userInfo = await dheClient.getUserInfo(); + const operateAs = userInfo.operateAs; + const excludeSet = new Set(exclude); + + return dheClient + .getKnownConfigs() + .filter( + qi => + isAttachableWorker(qi, operateAs) && + !excludeSet.has(qi.serial as QuerySerial) + ); +} + +/** + * Convert a QueryInfo object to a WorkerInfo object without any I/O or + * side effects. Returns undefined when `queryInfo.designated` is null. + * @param tagId Unique tag id to associate with the worker. + * @param queryInfo The running query info. + * @returns WorkerInfo or undefined. + */ +export function getWorkerInfoFromQueryInfo( + tagId: UniqueID, + queryInfo: QueryInfo +): WorkerInfo | undefined { + if (queryInfo.designated == null) { + return; + } + + assertDefined( + queryInfo.designated.ideUrl, + 'designated.ideUrl must be defined' + ); + + const { envoyPrefix, grpcUrl, ideUrl, jsApiUrl, processInfoId, workerName } = + queryInfo.designated; + + const workerUrl = new URL(jsApiUrl) as WorkerURL; + workerUrl.pathname = workerUrl.pathname.replace(/jsapi\/dh-core.js$/, ''); + + return { + tagId, + serial: queryInfo.serial as QuerySerial, + envoyPrefix, + grpcUrl: new URL(grpcUrl) as GrpcURL, + ideUrl: new URL(ideUrl) as IdeURL, + jsapiUrl: new URL(jsApiUrl) as JsapiURL, + name: queryInfo.name, + processInfoId, + workerName, + workerUrl, + }; +} + /** * Search existing queries for a query with the given tag id and return its serial. * @param tagId Unique tag id to search for. @@ -436,7 +519,7 @@ export function getSerialFromTagId( * @param querySerial Serial of the query to get worker info for. * @returns A promise that resolves to the worker info when the worker is ready. */ -export async function getWorkerInfoFromQuery( +export async function getWorkerInfoFromQuerySerial( tagId: UniqueID, dhe: DheType, dheClient: DheAuthenticatedClient, @@ -498,27 +581,7 @@ export async function getWorkerInfoFromQuery( } } - if (queryInfo.designated == null) { - return; - } - - const { envoyPrefix, grpcUrl, ideUrl, jsApiUrl, processInfoId, workerName } = - queryInfo.designated; - - const workerUrl = new URL(jsApiUrl) as WorkerURL; - workerUrl.pathname = workerUrl.pathname.replace(/jsapi\/dh-core.js$/, ''); - - return { - tagId, - serial: querySerial, - envoyPrefix, - grpcUrl: new URL(grpcUrl) as GrpcURL, - ideUrl: new URL(ideUrl) as IdeURL, - jsapiUrl: new URL(jsApiUrl) as JsapiURL, - processInfoId, - workerName, - workerUrl, - }; + return getWorkerInfoFromQueryInfo(tagId, queryInfo); } /** diff --git a/src/dh/modules.d.ts b/src/dh/modules.d.ts index d7de75a7b..ea6f96fce 100644 --- a/src/dh/modules.d.ts +++ b/src/dh/modules.d.ts @@ -1,12 +1,3 @@ -/** - * Augment types that are missing in current jsapi-types. - */ -declare module '@deephaven-enterprise/jsapi-types' { - interface EnterpriseClient { - deleteQueries(querySerials: string[]): Promise; - } -} - export {}; /** diff --git a/src/mcp/tools/listConnections.spec.ts b/src/mcp/tools/listConnections.spec.ts index 69b4fd704..5a8bcbab1 100644 --- a/src/mcp/tools/listConnections.spec.ts +++ b/src/mcp/tools/listConnections.spec.ts @@ -18,6 +18,7 @@ vi.mock('vscode'); const MOCK_CONNECTION_1: ConnectionState = { serverUrl: MOCK_DHC_URL, + label: 'Connection 1', isConnected: true, isRunningCode: false, tagId: 'conn1' as UniqueID, @@ -25,6 +26,7 @@ const MOCK_CONNECTION_1: ConnectionState = { const MOCK_CONNECTION_2: ConnectionState = { serverUrl: new URL('http://localhost:10001'), + label: 'Connection 2', isConnected: true, isRunningCode: true, tagId: 'conn2' as UniqueID, diff --git a/src/mcp/tools/listConnections.ts b/src/mcp/tools/listConnections.ts index 734d4890b..2c471359d 100644 --- a/src/mcp/tools/listConnections.ts +++ b/src/mcp/tools/listConnections.ts @@ -26,6 +26,7 @@ const spec = { .array( z.object({ serverUrl: z.string(), + label: z.string(), isConnected: z.boolean(), isRunningCode: z.boolean().optional(), querySerial: z.string().optional(), @@ -66,7 +67,7 @@ export function createListConnectionsTool({ const connections = await Promise.all( rawConnections.map( - async ({ serverUrl, isConnected, isRunningCode, tagId }) => { + async ({ serverUrl, label, isConnected, isRunningCode, tagId }) => { // Get worker info to retrieve querySerial for DHE connections const workerInfo = await serverManager.getWorkerInfo( serverUrl as WorkerURL @@ -74,6 +75,7 @@ export function createListConnectionsTool({ return { serverUrl: serverUrl.toString(), + label, isConnected, isRunningCode, tagId, diff --git a/src/mcp/utils/serverUtils.spec.ts b/src/mcp/utils/serverUtils.spec.ts index b63df4b8b..64f5a20c5 100644 --- a/src/mcp/utils/serverUtils.spec.ts +++ b/src/mcp/utils/serverUtils.spec.ts @@ -42,11 +42,13 @@ describe('serverUtils', () => { isRunningCode, serverUrl, tagId, + label: 'mock connection', }); expect(resultWithoutTag, 'without tag').toEqual({ isConnected, isRunningCode, + label: 'mock connection', serverUrl: serverUrl.toString(), tagId, }); @@ -389,6 +391,7 @@ describe('serverUtils', () => { const MOCK_CONNECTION: ConnectionState = { isConnected: true, isRunningCode: true, + label: 'mock connection', serverUrl, tagId, } as ConnectionState; diff --git a/src/mcp/utils/serverUtils.ts b/src/mcp/utils/serverUtils.ts index 26e5561ac..8e060d876 100644 --- a/src/mcp/utils/serverUtils.ts +++ b/src/mcp/utils/serverUtils.ts @@ -14,6 +14,7 @@ import { createConnectionNotFoundHint } from './runCodeUtils'; export const connectionResultSchema = z.object({ isConnected: z.boolean(), isRunningCode: z.boolean().optional(), + label: z.string(), serverUrl: z.string(), tagId: z.string().optional(), }); @@ -57,12 +58,14 @@ export type GetFirstConnectionOrCreateResult = export function connectionToResult({ isConnected, isRunningCode, + label, serverUrl, tagId, }: ConnectionState): ConnectionResult { return { isConnected, isRunningCode, + label, serverUrl: serverUrl.toString(), tagId, }; diff --git a/src/providers/ServerConnectionPanelTreeProvider.ts b/src/providers/ServerConnectionPanelTreeProvider.ts index 6f602be36..98369df70 100644 --- a/src/providers/ServerConnectionPanelTreeProvider.ts +++ b/src/providers/ServerConnectionPanelTreeProvider.ts @@ -2,17 +2,18 @@ import * as vscode from 'vscode'; import type { IPanelService, IServerManager, - ConnectionState, ServerConnectionPanelNode, } from '../types'; import { ServerTreeProviderBase } from './ServerTreeProviderBase'; import { + getConnectionServerTreeItem, + getConnectionTreeRootNodes, getPanelConnectionTreeItem, getPanelVariableTreeItem, + isServerStateNode, sortByStringProp, } from '../util'; import { getFirstSupportedConsoleType } from '../services'; -import { getServerMatchPortIfLocalHost } from '../mcp/utils'; export class ServerConnectionPanelTreeProvider extends ServerTreeProviderBase { constructor(serverManager: IServerManager, panelService: IPanelService) { @@ -27,35 +28,48 @@ export class ServerConnectionPanelTreeProvider extends ServerTreeProviderBase => { - if (Array.isArray(connectionOrVariable)) { - return getPanelVariableTreeItem(connectionOrVariable); + // Variable leaf node. + if (Array.isArray(node)) { + return getPanelVariableTreeItem(node); } - const serverLabel = getServerMatchPortIfLocalHost( - this.serverManager, - connectionOrVariable.serverUrl - )?.label; + // DHE server node grouping its worker connections. + if (isServerStateNode(node)) { + return getConnectionServerTreeItem(node); + } return getPanelConnectionTreeItem( - connectionOrVariable, + node, getFirstSupportedConsoleType, - serverLabel + node.label ); }; getChildren = ( - connectionOrRoot?: ConnectionState + elementOrRoot?: ServerConnectionPanelNode ): vscode.ProviderResult => { - if (connectionOrRoot == null) { + // Root: one server node per server that has connections. + if (elementOrRoot == null) { + return getConnectionTreeRootNodes(this.serverManager); + } + + // Variable leaf nodes have no children. + if (Array.isArray(elementOrRoot)) { + return []; + } + + // Server node -> its worker connections. + if (isServerStateNode(elementOrRoot)) { return this.serverManager - .getConnections() + .getConnections(elementOrRoot.url) .sort(sortByStringProp('serverUrl')); } - return [...this._panelService.getVariables(connectionOrRoot.serverUrl)] + // Connection node -> its panel variables. + return [...this._panelService.getVariables(elementOrRoot.serverUrl)] .sort(sortByStringProp('title')) - .map(variable => [connectionOrRoot.serverUrl, variable]); + .map(variable => [elementOrRoot.serverUrl, variable]); }; } diff --git a/src/providers/ServerConnectionTreeProvider.ts b/src/providers/ServerConnectionTreeProvider.ts index 3815937b6..d6fd7654a 100644 --- a/src/providers/ServerConnectionTreeProvider.ts +++ b/src/providers/ServerConnectionTreeProvider.ts @@ -1,90 +1,94 @@ import * as vscode from 'vscode'; import { ServerTreeProviderBase } from './ServerTreeProviderBase'; import { CONNECTION_TREE_ITEM_CONTEXT, ICON_ID } from '../common'; -import type { - IDhcService, - ConnectionState, - ServerConnectionNode, -} from '../types'; -import { isInstanceOf, sortByStringProp } from '../util'; +import type { ConsoleType, ServerConnectionNode } from '../types'; +import { + getConnectionServerTreeItem, + getConnectionTreeRootNodes, + getConsoleTypeIconId, + isInstanceOf, + isServerStateNode, + sortByStringProp, +} from '../util'; import { DhcService } from '../services'; -import { getServerMatchPortIfLocalHost } from '../mcp/utils'; /** * Provider for the server connection tree view. */ export class ServerConnectionTreeProvider extends ServerTreeProviderBase { getTreeItem = async ( - connectionOrUri: ServerConnectionNode + node: ServerConnectionNode ): Promise => { // Uri node associated with a parent connection node - if (connectionOrUri instanceof vscode.Uri) { + if (node instanceof vscode.Uri) { return { - description: connectionOrUri.path, + description: node.path, contextValue: CONNECTION_TREE_ITEM_CONTEXT.isUri, command: { command: 'vscode.open', title: 'Open Uri', - arguments: [connectionOrUri], + arguments: [node], }, - resourceUri: connectionOrUri, + resourceUri: node, }; } - const descriptionTokens: string[] = []; - - if ( - isInstanceOf(connectionOrUri, DhcService) && - connectionOrUri.isInitialized - ) { - const [consoleType] = await connectionOrUri.getConsoleTypes(); - if (consoleType) { - descriptionTokens.push(consoleType); - } + // DHE server node grouping its worker connections. + if (isServerStateNode(node)) { + return getConnectionServerTreeItem(node); } - if (connectionOrUri.tagId) { - descriptionTokens.push(connectionOrUri.tagId); + // Console type (language) drives the node icon rather than the description. + let consoleType: ConsoleType | undefined; + if (isInstanceOf(node, DhcService) && node.isInitialized) { + [consoleType] = await node.getConsoleTypes(); } - const hasUris = this.serverManager.hasConnectionUris(connectionOrUri); - - const serverLabel = getServerMatchPortIfLocalHost( - this.serverManager, - connectionOrUri.serverUrl - )?.label; + const hasUris = this.serverManager.hasConnectionUris(node); - const label = serverLabel ?? connectionOrUri.serverUrl.host; + // Identify connections created by the extension or in-flight placeholders + const isOwned = isInstanceOf(node, DhcService) ? node.isOwned : true; // Connection node return { - label, - description: descriptionTokens.join(' - '), - contextValue: connectionOrUri.isConnected - ? CONNECTION_TREE_ITEM_CONTEXT.isConnectionConnected - : CONNECTION_TREE_ITEM_CONTEXT.isConnectionConnecting, + label: node.label, + contextValue: node.isConnected + ? CONNECTION_TREE_ITEM_CONTEXT.isConnectionConnected(isOwned) + : CONNECTION_TREE_ITEM_CONTEXT.isConnectionConnecting(isOwned), collapsibleState: hasUris ? vscode.TreeItemCollapsibleState.Expanded : undefined, + // Show the language (Python/Groovy) icon when idle/connected; show the + // spinner while busy (connecting or running code). iconPath: new vscode.ThemeIcon( - connectionOrUri.isRunningCode - ? ICON_ID.runningCode - : connectionOrUri.isConnected - ? ICON_ID.connected - : ICON_ID.connecting + node.isRunningCode || !node.isConnected + ? ICON_ID.connecting + : getConsoleTypeIconId(consoleType) ), }; }; getChildren = ( - elementOrRoot?: IDhcService + elementOrRoot?: ServerConnectionNode ): vscode.ProviderResult => { + // Root: one server node per server that has connections. if (elementOrRoot == null) { + return getConnectionTreeRootNodes(this.serverManager); + } + + // Uri leaf nodes have no children. + if (elementOrRoot instanceof vscode.Uri) { + return []; + } + + // Server node -> its worker connections. + if (isServerStateNode(elementOrRoot)) { return this.serverManager - .getConnections() + .getConnections(elementOrRoot.url) .sort(sortByStringProp('serverUrl')); } + // Connection node -> its editor uris. return this.serverManager.getConnectionUris(elementOrRoot); }; @@ -93,11 +97,16 @@ export class ServerConnectionTreeProvider extends ServerTreeProviderBase { + getParent = (element: ServerConnectionNode): ServerConnectionNode | null => { if (element instanceof vscode.Uri) { return this.serverManager.getUriConnection(element); } - return null; + if (isServerStateNode(element)) { + return null; + } + + // Connection node -> its parent server. + return this.serverManager.getServerForConnection(element) ?? null; }; } diff --git a/src/providers/ServerTreeProvider.ts b/src/providers/ServerTreeProvider.ts index 629382f53..8eafaaad3 100644 --- a/src/providers/ServerTreeProvider.ts +++ b/src/providers/ServerTreeProvider.ts @@ -21,7 +21,10 @@ export class ServerTreeProvider extends ServerTreeProviderBase { return getServerGroupTreeItem(element, this.serverManager.canStartServer); } - return getServerTreeItem(element); + return getServerTreeItem( + element, + this.serverManager.isServerConnecting(element.url) + ); }; getChildren(elementOrRoot?: ServerNode): vscode.ProviderResult { diff --git a/src/services/DhcService.ts b/src/services/DhcService.ts index 3c4002451..6f4af7763 100644 --- a/src/services/DhcService.ts +++ b/src/services/DhcService.ts @@ -72,9 +72,16 @@ export class DhcService extends DisposableBase implements IDhcService { toaster: IToastService ): IDhcServiceFactory => { return { - create: (serverUrl: URL, tagId?: UniqueID): IDhcService => { + create: ( + label: string, + serverUrl: URL, + isOwned: boolean, + tagId?: UniqueID + ): IDhcService => { return new DhcService( + label, serverUrl, + isOwned, coreClientCache, groovyDiagnosticsCollection, diagnosticsCollection, @@ -94,7 +101,9 @@ export class DhcService extends DisposableBase implements IDhcService { * mechanism for instantiating. */ private constructor( + label: string, serverUrl: URL, + isOwned: boolean, coreClientCache: URLMap, groovyDiagnosticsCollection: vscode.DiagnosticCollection, diagnosticsCollection: vscode.DiagnosticCollection, @@ -108,6 +117,8 @@ export class DhcService extends DisposableBase implements IDhcService { super(); this.coreClientCache = coreClientCache; + this.isOwned = isOwned; + this.label = label; this.groovyDiagnosticsCollection = groovyDiagnosticsCollection; this.diagnosticsCollection = diagnosticsCollection; this.remoteFileSourceService = remoteFileSourceService; @@ -132,6 +143,8 @@ export class DhcService extends DisposableBase implements IDhcService { private readonly _onDidDisconnect = new vscode.EventEmitter(); readonly onDidDisconnect = this._onDidDisconnect.event; + public readonly isOwned: boolean; + public readonly label: string; public readonly serverUrl: URL; public readonly tagId?: UniqueID; diff --git a/src/services/DheService.ts b/src/services/DheService.ts index 79b504668..300e862d7 100644 --- a/src/services/DheService.ts +++ b/src/services/DheService.ts @@ -18,14 +18,17 @@ import { type WorkerInfo, type WorkerURL, } from '../types'; -import { Logger, URLMap } from '../util'; +import { Logger, uniqueId, URLMap } from '../util'; import { + getWorkerInfoFromQueryInfo, createInteractiveConsoleQuery, createQueryName, deleteQueries, getDheFeatures, getSerialFromTagId, - getWorkerInfoFromQuery, + getWorkerInfoFromQuerySerial, + isAttachableWorker, + listAttachableWorkers, } from '../dh/dhe'; import { CLOSE_CREATE_QUERY_VIEW_CMD, @@ -61,15 +64,21 @@ export class DheService implements IDheService { toaster: IToastService ): IDheServiceFactory => { return { - create: (serverUrl: URL): IDheService => - new DheService( + create: (serverUrl: URL): IDheService => { + const serverConfig = configService + .getEnterpriseServers() + .find(server => server.url.href === serverUrl.href); + + return new DheService( + serverConfig?.label ?? serverUrl.href, serverUrl, configService, dheClientCache, dheJsApiCache, interactiveConsoleQueryFactory, toaster - ), + ); + }, }; }; @@ -78,6 +87,7 @@ export class DheService implements IDheService { * mechanism for instantiating. */ private constructor( + label: string, serverUrl: URL, configService: IConfigService, dheClientCache: URLMap, @@ -85,6 +95,7 @@ export class DheService implements IDheService { interactiveConsoleQueryFactory: IInteractiveConsoleQueryFactory, toaster: IToastService ) { + this.label = label; this.serverUrl = serverUrl; this._config = configService; this._dheClientCache = dheClientCache; @@ -101,18 +112,26 @@ export class DheService implements IDheService { private _clientPromise: Promise | null = null; private _isConnected: boolean = false; + private _operateAs: string | null = null; + private _removeConfigListeners: (() => void) | null = null; + private readonly _config: IConfigService; private readonly _dheClientCache: URLMap; private readonly _dheJsApiCache: IAsyncCacheService; private readonly _dheServerFeaturesCache: URLMap; + private readonly _pendingQueryTagIds = new Set(); private readonly _querySerialSet: Set; private readonly _interactiveConsoleQueryFactory: IInteractiveConsoleQueryFactory; private readonly _toaster: IToastService; private readonly _workerInfoMap: URLMap; - private readonly _onDidWorkerTerminate = new vscode.EventEmitter(); - readonly onDidWorkerTerminate = this._onDidWorkerTerminate.event; + private readonly _onWorkerAttachable = new vscode.EventEmitter(); + readonly onWorkerAttachable = this._onWorkerAttachable.event; + + private readonly _onWorkerRemoved = new vscode.EventEmitter(); + readonly onWorkerRemoved = this._onWorkerRemoved.event; + readonly label: string; readonly serverUrl: URL; /** @@ -141,7 +160,9 @@ export class DheService implements IDheService { const maybeClient = await this._dheClientCache.get(this.serverUrl); if (maybeClient != null) { - this._subscribeToWorkerTermination(maybeClient); + const userInfo = await maybeClient.client.getUserInfo(); + this._operateAs = userInfo.operateAs; + await this._subscribeToWorkerEvents(maybeClient); } if (!this._dheServerFeaturesCache.has(this.serverUrl)) { @@ -177,7 +198,18 @@ export class DheService implements IDheService { }; private _onDidDheClientCacheInvalidate = (url: URL): void => { - if (url.toString() === this.serverUrl.toString()) { + // Only reset when the client was actually removed from the cache (logout / + // disconnect / failed login). The cache also fires `onDidChange` on `set`, + // which happens during a successful login from inside `_initClient` itself. + // Resetting on that `set` would null out the in-flight `_clientPromise` we + // are currently awaiting, so a subsequent `getClient(false)` (e.g. from + // `listAttachableWorkers`) would see `null` and short-circuit — silently + // skipping the attach path. Guard on the client being absent so we only + // reset for genuine invalidations. + if ( + url.toString() === this.serverUrl.toString() && + !this._dheClientCache.has(this.serverUrl) + ) { // Reset the client promise so that the next call to `getClient` can // reinitialize it if necessary. this._clientPromise = null; @@ -185,36 +217,79 @@ export class DheService implements IDheService { }; /** - * Subscribe to DHE config updates to detect when workers enter a terminal state. + * Subscribe to DHE config added/updated/removed events. Emits + * `onDidWorkerAttachable` when an IC worker becomes attachable, and + * `onDidWorkerRemoved` when a tracked worker is removed or enters a terminal + * state. Replaces any previous subscription. * @param dheClient DHE client to use. */ - private _subscribeToWorkerTermination = async ( + private _subscribeToWorkerEvents = async ( dheClient: DheAuthenticatedClientWrapper ): Promise => { + // Remove any previous listeners before re-registering. + this._removeConfigListeners?.(); + const dhe = await this._dheJsApiCache.get(this.serverUrl); - dheClient.client.addEventListener( - dhe.Client.EVENT_CONFIG_UPDATED, - ({ detail: queryInfo }: CustomEvent) => { - const status = queryInfo.designated?.status; - if (!isTerminalQueryStatus(status)) { - return; + const onConfigAddedOrUpdated = ({ + detail: queryInfo, + }: CustomEvent): void => { + const status = queryInfo.designated?.status; + + if (isTerminalQueryStatus(status)) { + // Terminal status on a tracked worker, fire _onWorkerRemoved + if (this._isQueryTracked(queryInfo)) { + logger.info('Worker entered terminal state:', queryInfo.serial); + this._onWorkerRemoved.fire(queryInfo.serial as QuerySerial); } - const workerInfo = [...this._workerInfoMap.values()].find( - w => w.serial === queryInfo.serial - ); + return; + } - if (workerInfo == null) { - return; - } + if ( + !this._isQueryOwned(queryInfo) && + isAttachableWorker(queryInfo, this._operateAs) + ) { + this._onWorkerAttachable.fire(queryInfo); + } + }; - logger.info( - 'Worker entered terminal state:', - workerInfo.workerUrl.href - ); - this._onDidWorkerTerminate.fire(workerInfo.workerUrl); + const onConfigRemoved = ({ + detail: queryInfo, + }: CustomEvent): void => { + if (!this._isQueryTracked(queryInfo)) { + return; } + + logger.info('Worker removed:', queryInfo.serial); + this._onWorkerRemoved.fire(queryInfo.serial as QuerySerial); + }; + + const removeConfigListeners: (() => void)[] = []; + this._removeConfigListeners = (): void => { + for (const remove of removeConfigListeners) { + remove(); + } + removeConfigListeners.length = 0; + }; + + removeConfigListeners.push( + dheClient.client.addEventListener( + dhe.Client.EVENT_CONFIG_ADDED, + onConfigAddedOrUpdated + ) + ); + removeConfigListeners.push( + dheClient.client.addEventListener( + dhe.Client.EVENT_CONFIG_UPDATED, + onConfigAddedOrUpdated + ) + ); + removeConfigListeners.push( + dheClient.client.addEventListener( + dhe.Client.EVENT_CONFIG_REMOVED, + onConfigRemoved + ) ); }; @@ -273,6 +348,34 @@ export class DheService implements IDheService { return dheClient; } + private _isQueryOwned = ( + querySerialOrInfo: QuerySerial | QueryInfo + ): boolean => { + const { name, serial } = + typeof querySerialOrInfo === 'object' + ? querySerialOrInfo + : { serial: querySerialOrInfo }; + + if (name != null) { + for (const tagId of this._pendingQueryTagIds) { + // Created workers are named `IC - VS Code - ` (the iframe create + // flow may append a `_vN` version suffix), so a prefix match against + // in-flight tagIds covers both forms + const namePrefix = createQueryName(tagId); + + if (name.startsWith(namePrefix)) { + return true; + } + } + } + + return this._querySerialSet.has(serial as QuerySerial); + }; + + private _isQueryTracked = ({ serial }: QueryInfo): boolean => { + return [...this._workerInfoMap.values()].some(w => w.serial === serial); + }; + /** * Create an InteractiveConsole query and get worker info from it. * @param tagId Unique tag id to include in the worker info. @@ -300,6 +403,15 @@ export class DheService implements IDheService { let startupFailureStatus: string | null = null; const queryName = createQueryName(tagId); + + // Suppress auto-attach for this worker while it is being created. The + // config event that flips it to `Running` would otherwise race the create + // path and auto-attach it (see `onDidWorkerAttachable` guard). Cleared in + // the `finally` below, by which point its serial is in `_querySerialSet` + // (added synchronously right after, with no `await` in between), so the + // serial-based guard takes over without a gap. + this._pendingQueryTagIds.add(tagId); + const removeStartupFailureListener = dheClient.client.addEventListener( dhe.Client.EVENT_CONFIG_UPDATED, ({ detail: queryInfo }: CustomEvent) => { @@ -348,6 +460,7 @@ export class DheService implements IDheService { throw err; } finally { removeStartupFailureListener(); + this._pendingQueryTagIds.delete(tagId); } if (querySerial == null) { @@ -355,7 +468,7 @@ export class DheService implements IDheService { } this._querySerialSet.add(querySerial); - const workerInfo = await getWorkerInfoFromQuery( + const workerInfo = await getWorkerInfoFromQuerySerial( tagId, dhe, dheClient.client, @@ -371,7 +484,46 @@ export class DheService implements IDheService { }; /** - * Delete a worker. + * Register a pre-existing Running worker by building WorkerInfo from the + * given QueryInfo. Does NOT add the serial to `_querySerialSet` — registered + * workers are never owned and must never be deleted by the extension. + * @param queryInfo The already-Running QueryInfo for the worker to register. + * @returns WorkerInfo for the registered worker. + */ + registerWorkerInfo = (queryInfo: QueryInfo): WorkerInfo => { + const tagId = uniqueId(); + const workerInfo = getWorkerInfoFromQueryInfo(tagId, queryInfo); + if (workerInfo == null) { + throw new Error( + `Cannot register worker for query info: ${queryInfo.serial}` + ); + } + + this._workerInfoMap.set(workerInfo.workerUrl, workerInfo); + + return workerInfo; + }; + + /** + * List all running InteractiveConsole workers owned by the current effective + * user. + * @param exclude Iterable of query serials to exclude from the results. + * @returns A promise resolving to the filtered QueryInfo array. + */ + listAttachableWorkers = async ( + exclude: Iterable + ): Promise => { + const dheClient = await this.getClient(false); + if (dheClient == null) { + return []; + } + return listAttachableWorkers(dheClient.client, exclude); + }; + + /** + * Delete a worker. Only deletes the server-side PQ when the worker is owned + * by this extension (serial is in `_querySerialSet`). Attached workers are + * removed from `_workerInfoMap` but the PQ is left running. * @param workerUrl Worker URL to delete. */ deleteWorker = async (workerUrl: WorkerURL): Promise => { @@ -380,10 +532,12 @@ export class DheService implements IDheService { return; } - this._querySerialSet.delete(workerInfo.serial); this._workerInfoMap.delete(workerUrl); - await this._disposeQueries([workerInfo.serial]); + if (this._isQueryOwned(workerInfo.serial)) { + this._querySerialSet.delete(workerInfo.serial); + await this._disposeQueries([workerInfo.serial]); + } }; getQuerySerialFromTag = async ( @@ -407,7 +561,10 @@ export class DheService implements IDheService { const querySerials = [...this._querySerialSet]; this._querySerialSet.clear(); - this._onDidWorkerTerminate.dispose(); + this._removeConfigListeners?.(); + this._removeConfigListeners = null; + this._onWorkerAttachable.dispose(); + this._onWorkerRemoved.dispose(); await Promise.all([ this._workerInfoMap.dispose(), diff --git a/src/services/ServerManager.spec.ts b/src/services/ServerManager.spec.ts index 8ae55c9a4..3205f41d0 100644 --- a/src/services/ServerManager.spec.ts +++ b/src/services/ServerManager.spec.ts @@ -1,7 +1,7 @@ import * as vscode from 'vscode'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { ServerManager } from './ServerManager'; -import { URLMap, withResolvers } from '../util'; +import { URLMap, withResolvers, type PromiseWithResolvers } from '../util'; import type { ConnectionState, IAsyncCacheService, @@ -36,8 +36,14 @@ vi.mock('../dh/dhe', () => ({ type TestServerManager = PublicOf & { _serverMap: URLMap; _connectionMap: URLMap; - _pendingConnectionMap: URLMap>; - _doConnectToServer: ReturnType; + _pendingConnectionMap: URLMap>; + _pendingServerConnections: URLMap>; + _dhcServiceFactory: { create: ReturnType }; + _dheServiceCache: { + has: ReturnType; + get: ReturnType; + }; + _resolvePendingServerConnection: (serverUrl: URL) => void; }; /** Build a `ServerManager` with minimal mocked dependencies. */ @@ -71,7 +77,7 @@ function createServerManager(): TestServerManager { } function mockConnectionState(url: URL): ConnectionState { - return { isConnected: true, serverUrl: url }; + return { label: 'Mock connection state', isConnected: true, serverUrl: url }; } function mockServerState({ @@ -107,21 +113,13 @@ const dheServer0 = mockServerState({ type: 'DHE', }); const cn1 = mockConnectionState(serverUrl); -const cn2 = mockConnectionState(serverUrl); describe('ServerManager.connectToServer', () => { let manager: TestServerManager; - let promise: Promise; - let resolve: (value: ConnectionState | null) => void; - beforeEach(() => { vi.clearAllMocks(); manager = createServerManager(); - - ({ promise, resolve } = withResolvers()); - - manager._doConnectToServer = vi.fn().mockReturnValue(promise); }); it('throws when the server is not found', async () => { @@ -137,50 +135,118 @@ describe('ServerManager.connectToServer', () => { const result = await manager.connectToServer(serverUrl); expect(result).toBe(cn1); - expect(manager._doConnectToServer).not.toHaveBeenCalled(); + expect(manager._dhcServiceFactory.create).not.toHaveBeenCalled(); }); it('only connects once when called concurrently for the same DHC server', async () => { manager._serverMap.set(dhcServer0.url, dhcServer0); + // Hold the client handshake open so the connection stays in flight. + const { promise: clientPromise, resolve: resolveClient } = + withResolvers(); + const connection = { + getClient: vi.fn().mockReturnValue(clientPromise), + initSession: vi.fn().mockResolvedValue(true), + onDidDisconnect: vi.fn(), + onDidChangeRunningCodeStatus: vi.fn(), + }; + manager._dhcServiceFactory.create.mockReturnValue(connection); + const first = manager.connectToServer(serverUrl); const second = manager.connectToServer(serverUrl); - // The in-progress connection is reused rather than starting a new one. - expect(manager._doConnectToServer).toHaveBeenCalledTimes(1); + // The second call dedupes against the in-flight connection rather than + // creating a new one. + expect(manager._dhcServiceFactory.create).toHaveBeenCalledTimes(1); + expect(manager.isServerConnecting(serverUrl)).toBe(true); - resolve(cn1); + resolveClient({}); - expect(await first).toBe(cn1); - expect(await second).toBe(cn1); + expect(await first).toBe(connection); + expect(await second).toBe(connection); }); - it('clears the pending connection once it resolves so later calls reconnect', async () => { - manager._serverMap.set(dhcServer0.url, dhcServer0); + it('dedups concurrent client connections for DHE servers', () => { + manager._serverMap.set(dheServer0.url, dheServer0); - manager._doConnectToServer.mockResolvedValue(cn1); - const first = manager.connectToServer(serverUrl); - expect(manager._pendingConnectionMap.has(serverUrl)).toBe(true); - expect(await first).toBe(cn1); + // Leave the DHE service acquisition pending so the client connection stays + // in flight. + const { promise } = withResolvers(); + manager._dheServiceCache.get.mockReturnValue(promise); - expect(manager._pendingConnectionMap.has(serverUrl)).toBe(false); + void manager.connectToServer(serverUrl); + void manager.connectToServer(serverUrl); - // A subsequent connect attempt is not blocked by the resolved pending entry. - manager._doConnectToServer.mockResolvedValue(cn2); - const second = manager.connectToServer(serverUrl); - expect(await second).toBe(cn2); + // The DHE client connection is singular, so concurrent attempts reuse the + // in-flight connection rather than starting a second one (multiple workers + // are created later, off the single client connection). + expect(manager._dheServiceCache.get).toHaveBeenCalledTimes(1); + expect(manager.isServerConnecting(serverUrl)).toBe(true); + }); +}); + +describe('ServerManager.isServerConnecting', () => { + let manager: TestServerManager; - expect(manager._doConnectToServer).toHaveBeenCalledTimes(2); + beforeEach(() => { + vi.clearAllMocks(); + manager = createServerManager(); }); - it('does not track pending connections for DHE servers', async () => { - manager._serverMap.set(dheServer0.url, dheServer0); + it('reflects a pending server connection and fires onDidUpdate when it resolves', () => { + const onDidUpdate = vi.fn(); + manager.onDidUpdate(onDidUpdate); - void manager.connectToServer(serverUrl); - void manager.connectToServer(serverUrl); + expect(manager.isServerConnecting(serverUrl)).toBe(false); + + manager._pendingServerConnections.set(serverUrl, withResolvers()); + expect(manager.isServerConnecting(serverUrl)).toBe(true); + + manager._resolvePendingServerConnection(serverUrl); + expect(manager.isServerConnecting(serverUrl)).toBe(false); + expect(onDidUpdate).toHaveBeenCalledTimes(1); + }); + + it('resolves idempotently and does not fire when there is no pending entry', () => { + const onDidUpdate = vi.fn(); + manager.onDidUpdate(onDidUpdate); + + // Resolving when not connecting is a no-op (no fire). + manager._resolvePendingServerConnection(serverUrl); + expect(onDidUpdate).not.toHaveBeenCalled(); + + manager._pendingServerConnections.set(serverUrl, withResolvers()); + manager._resolvePendingServerConnection(serverUrl); + expect(onDidUpdate).toHaveBeenCalledTimes(1); + + // Resolving again is a no-op (no extra fire). + manager._resolvePendingServerConnection(serverUrl); + expect(onDidUpdate).toHaveBeenCalledTimes(1); + }); + + it('clears the pending entry when a connect settles, so a retry is not blocked', async () => { + manager._serverMap.set(dhcServer0.url, dhcServer0); - // DHE supports multiple connections, so concurrent calls each start one. - expect(manager._doConnectToServer).toHaveBeenCalledTimes(2); - expect(manager._pendingConnectionMap.has(serverUrl)).toBe(false); + // First attempt fails to get a client; `_doConnectToServer`'s `finally` + // must clear the pending entry (no stale entry left behind). + const failConnection = { getClient: vi.fn().mockResolvedValue(null) }; + const okConnection = { + getClient: vi.fn().mockResolvedValue({}), + initSession: vi.fn().mockResolvedValue(true), + onDidDisconnect: vi.fn(), + onDidChangeRunningCodeStatus: vi.fn(), + }; + manager._dhcServiceFactory.create + .mockReturnValueOnce(failConnection) + .mockReturnValueOnce(okConnection); + + expect(await manager.connectToServer(serverUrl)).toBeNull(); + expect(manager.isServerConnecting(serverUrl)).toBe(false); + + // The retry is not blocked by a stale pending entry — it starts a new + // connection rather than deduping against the failed one. + expect(await manager.connectToServer(serverUrl)).toBe(okConnection); + expect(manager._dhcServiceFactory.create).toHaveBeenCalledTimes(2); + expect(manager.isServerConnecting(serverUrl)).toBe(false); }); }); diff --git a/src/services/ServerManager.ts b/src/services/ServerManager.ts index 334b5029c..093ea2f36 100644 --- a/src/services/ServerManager.ts +++ b/src/services/ServerManager.ts @@ -1,6 +1,7 @@ import * as vscode from 'vscode'; import { randomUUID } from 'node:crypto'; import type { dh as DhcType } from '@deephaven/jsapi-types'; +import type { QueryInfo } from '@deephaven-enterprise/jsapi-types'; import { QueryCreationCancelledError, QueryStartupFailureError, @@ -32,9 +33,12 @@ import { uniqueId, URIMap, URLMap, + withResolvers, + type PromiseWithResolvers, } from '../util'; import { DhcService } from './DhcService'; import { getWorkerCredentials, isDheServerRunning } from '../dh/dhe'; +import type { QuerySerial } from '../shared'; import { isDhcServerRunning } from '../dh/dhc'; const logger = new Logger('ServerManager'); @@ -52,7 +56,10 @@ export class ServerManager implements IServerManager { ) { this._configService = configService; this._connectionMap = new URLMap(); - this._pendingConnectionMap = new URLMap>(); + this._pendingConnectionMap = new URLMap< + PromiseWithResolvers + >(); + this._pendingServerConnections = new URLMap>(); this._coreClientCache = coreClientCache; this._dhcServiceFactory = dhcServiceFactory; this._dheClientCache = dheClientCache; @@ -69,10 +76,15 @@ export class ServerManager implements IServerManager { void this.loadServerConfig(); } + private readonly _attachedWorkerSerials: Map = + new Map(); private readonly _configService: IConfigService; private readonly _connectionMap: URLMap; private readonly _pendingConnectionMap: URLMap< - Promise + PromiseWithResolvers + >; + private readonly _pendingServerConnections: URLMap< + PromiseWithResolvers >; private readonly _coreClientCache: URLMap; private readonly _dhcServiceFactory: IDhcServiceFactory; @@ -104,6 +116,28 @@ export class ServerManager implements IServerManager { private readonly _onDidUpdate = new vscode.EventEmitter(); readonly onDidUpdate = this._onDidUpdate.event; + private _resolvePendingConnection = ( + serverUrl: URL, + connectionState: ConnectionState | null + ): void => { + if (this._pendingConnectionMap.has(serverUrl)) { + this._pendingConnectionMap.getOrThrow(serverUrl).resolve(connectionState); + this._pendingConnectionMap.delete(serverUrl); + this._onDidUpdate.fire(); + } + }; + + private _resolvePendingServerConnection = (serverUrl: URL): void => { + if (this._pendingServerConnections.has(serverUrl)) { + this._pendingServerConnections.getOrThrow(serverUrl).resolve(); + this._pendingServerConnections.delete(serverUrl); + this._onDidUpdate.fire(); + } + }; + + isServerConnecting = (serverUrl: URL): boolean => + this._pendingServerConnections.has(serverUrl); + private _lastServerRunningStatus = new URLMap(); private _hasEverUpdatedStatus = false; @@ -197,6 +231,16 @@ export class ServerManager implements IServerManager { this._onDidLoadConfig.fire(); }; + /** + * Connect to a server and attach to any Core / Core+ workers available for + * the server. For DHE, if no workers are available, creates one and attaches + * to it. + * @param serverUrl + * @param workerConsoleType + * @param operateAsAnotherUser + * @returns The connection state of the attached worker, or `null` if the + * connection or worker creation/attachment failed. + */ connectToServer = async ( serverUrl: URL, workerConsoleType?: ConsoleType, @@ -208,7 +252,7 @@ export class ServerManager implements IServerManager { throw new Error(`Server with URL '${serverUrl}' not found.`); } - // We only support 1 connection for DHC servers in the extension + // We only support 1 connection to a DHC server if (serverState.type === 'DHC' && serverState.connectionCount > 0) { logger.info('Already connected to server:', serverUrl.href); return this._connectionMap.getOrThrow(serverUrl); @@ -216,144 +260,135 @@ export class ServerManager implements IServerManager { if (this._pendingConnectionMap.has(serverUrl)) { logger.debug('Connection already in progress:', serverUrl.href); - return this._pendingConnectionMap.getOrThrow(serverUrl); + return this._pendingConnectionMap.getOrThrow(serverUrl).promise; } - const connectionPromise = this._doConnectToServer( + return this._doConnectToServer( serverState, workerConsoleType, operateAsAnotherUser ); - - // We only support 1 connection for DHC servers in the extension, but the - // count doesn't get updated until the connection is established, so we need - // to mark pending connections to prevent multiple simultaneous connection - // attempts to the same DHC server. - if (serverState.type === 'DHC') { - this._pendingConnectionMap.set( - serverUrl, - connectionPromise.then(result => { - this._pendingConnectionMap.delete(serverUrl); - return result; - }) - ); - } - - return connectionPromise; }; private _doConnectToServer = async ( serverState: ServerState, - workerConsoleType?: ConsoleType, - operateAsAnotherUser: boolean = false + workerConsoleType: ConsoleType | undefined = undefined, + operateAsAnotherUser: boolean ): Promise => { - let serverUrl = serverState.url; + const serverUrl = serverState.url; logger.debug('Connecting to server:', serverUrl.href); - let tagId: UniqueID | undefined; - - let placeholderUrl: URL | undefined; - - if (serverState.type === 'DHE') { - const isNewDheService = !this._dheServiceCache.has(serverUrl); - const dheService = await this._dheServiceCache.get(serverUrl); + // Mark server and worker connection as pending + this._pendingServerConnections.set(serverUrl, withResolvers()); + this._pendingConnectionMap.set(serverUrl, withResolvers()); + this._onDidUpdate.fire(); - // Get client. Client will be initialized if it doesn't exist (including - // prompting user for login). - if (!(await dheService.getClient(true, operateAsAnotherUser))) { - return null; - } + let firstConnection: ConnectionState | null = null; - // Handle workers stopped externally (i.e. web-client Query Monitor) - if (isNewDheService) { - dheService.onDidWorkerTerminate(workerUrl => { - this.disconnectFromServer(workerUrl); - }); - } + try { + if (serverState.type === 'DHC') { + // DHC attach to Core worker + firstConnection = await this._attachToWorker('Core', serverUrl, true); - // The `serverUrl` in this block is for the DHE server but gets set to the - // newly created worker url before leaving the block, so we need to update - // the connection count while we still have the reference. - this.updateConnectionCount(serverUrl, 1); - tagId = uniqueId(); + this._resolvePendingServerConnection(serverUrl); + } else { + const dheService = await this._connectToDheServer( + serverUrl, + operateAsAnotherUser + ); - // Put a placeholder connection in place until the worker is ready. - placeholderUrl = this.addWorkerPlaceholderConnection(serverUrl, tagId); + // server connection is done + this._resolvePendingServerConnection(serverUrl); - let workerInfo: WorkerInfo; - try { - workerInfo = await dheService.createWorker(tagId, workerConsoleType); - - // If the worker finished creating, but there is no placeholder connection, - // this indicates that the user cancelled the creation before it was ready. - // In this case, dispose of the worker. - if (!this._connectionMap.has(placeholderUrl)) { - dheService.deleteWorker(workerInfo.workerUrl); - this._onDidUpdate.fire(); + if (dheService == null) { return null; } - } catch (err) { - if (err instanceof QueryCreationCancelledError) { - logger.info(err); - const msg = 'Connection cancelled.'; - this._outputChannel.appendLine(msg); - this._toaster.info(msg); - } else { - const msg = - err instanceof QueryStartupFailureError - ? err.message - : 'Failed to create worker.'; - logger.error(err); - this._outputChannel.appendLine(msg); - this._toaster.error(msg); - } - this.updateConnectionCount(serverUrl, -1); - this._connectionMap.delete(placeholderUrl); - return null; + [firstConnection = null] = await this._createOrAttachToWorkers( + dheService, + workerConsoleType + ); + + return firstConnection; } + } finally { + this._resolvePendingConnection(serverUrl, firstConnection); + } - // Map the worker URL to the server URL to make things easier to dispose - // later. - this._workerURLToServerURLMap.set( - new URL(workerInfo.workerUrl), - serverUrl - ); + return firstConnection; + }; + + /** + * Create a Core+ JS API connection to an existing worker. Populates + * `_workerURLToServerURLMap` for auth lookup and `_attachedWorkerSerials` + * for idempotency/teardown. Used by both the create path and the attach path. + * Does NOT touch placeholder connections — that is the create path's concern. + * @param label The connection label to show in the UI for this worker. + * @param serverUrl The DHE server this worker belongs to. + * @param workerInfo Worker info built from the query. + * @returns The new connection state, or null on failure. + */ + private _attachToWorker = async ( + label: string, + serverUrl: URL, + isOwned: boolean, + workerInfo?: WorkerInfo + ): Promise => { + const workerUrl = + workerInfo == null ? serverUrl : new URL(workerInfo.workerUrl); + + if (workerInfo != null) { + // Idempotency gate: reserve the serial synchronously, before any `await`, + // so concurrent attach attempts for the same worker — e.g. the initial + // enumeration racing a streaming config event, or two clicks on the same + // server — cannot both connect and double-count. A later failure rolls the + // reservation back so the worker can be retried. + if (this._attachedWorkerSerials.has(workerInfo.serial)) { + return this._connectionMap.get(workerUrl) ?? null; + } + this._attachedWorkerSerials.set(workerInfo.serial, workerInfo.workerUrl); - // Update the server URL to the worker url to be used below with core - // connection creation. - serverUrl = new URL(workerInfo.workerUrl); + // Map the worker URL to its DHE server so the auth flow can resolve creds. + this._workerURLToServerURLMap.set(workerUrl, serverUrl); } - const connection = this._dhcServiceFactory.create(serverUrl, tagId); + const connection = this._dhcServiceFactory.create( + label, + workerUrl, + isOwned, + workerInfo?.tagId + ); - // Initialize client + prompt for login if necessary + // Initialize client (includes auth flow). const coreClient = await connection.getClient(); - // Cleanup placeholder connection if one exists - if (placeholderUrl) { - this.removeWorkerPlaceholderConnection(placeholderUrl); - } - if (coreClient == null) { + if (workerInfo != null) { + this._attachedWorkerSerials.delete(workerInfo.serial); + } + return null; } - this._connectionMap.set(serverUrl, connection); + this._connectionMap.set(workerUrl, connection); this._onDidUpdate.fire(); if (!(await connection.initSession())) { - this._coreClientCache.delete(serverUrl); + if (workerInfo != null) { + this._attachedWorkerSerials.delete(workerInfo.serial); + } + + this._coreClientCache.delete(workerUrl); connection.dispose(); - this._connectionMap.delete(serverUrl); + this._connectionMap.delete(workerUrl); return null; } connection.onDidDisconnect(() => { - logger.debug('onDidDisconnect fired for:', serverUrl.href); - this.disconnectFromServer(serverUrl); + logger.debug('onDidDisconnect fired for:', workerUrl.href); + this.disconnectFromServer(workerUrl); }); connection.onDidChangeRunningCodeStatus?.(() => { @@ -362,19 +397,247 @@ export class ServerManager implements IServerManager { this.updateConnectionCount(serverUrl, 1); - this._onDidConnect.fire(serverUrl); + this._onDidConnect.fire(workerUrl); this._onDidUpdate.fire(); - return this._connectionMap.get(serverUrl) ?? null; + return this._connectionMap.get(workerUrl) ?? null; + }; + + private _connectToDheServer = async ( + serverUrl: URL, + operateAsAnotherUser: boolean + ): Promise => { + const dheServerUrl = serverUrl; + const isNewDheService = !this._dheServiceCache.has(dheServerUrl); + const dheService = await this._dheServiceCache.get(dheServerUrl); + + // Get client. Client will be initialized if it doesn't exist (including + // prompting user for login). + if (!(await dheService.getClient(true, operateAsAnotherUser))) { + return null; + } + + // Mark the DHE server as connected now that we have a live client. + // (connectionCount stays 0 until workers attach; isConnected reflects + // the server-level connection, which is now established.) + const currentServerState = this._serverMap.get(dheServerUrl); + if (currentServerState != null && !currentServerState.isConnected) { + this._serverMap.set(dheServerUrl, { + ...currentServerState, + isConnected: true, + }); + this._onDidUpdate.fire(); + } + + // Wire the config-event subscription exactly once per DHE service + // instance, BEFORE snapshotting, so any worker that appears or disappears + // between the snapshot and now is not missed. `_connectWorker` reserves + // each serial synchronously (before any `await`), so an event-driven + // attach and the snapshot batch below can never double-connect the same + // worker — whichever reaches `_connectWorker` first wins and the other is + // a no-op. + if (isNewDheService) { + dheService.onWorkerAttachable(qi => + this._reconcileAttach(dheServerUrl, dheService, qi) + ); + dheService.onWorkerRemoved(serial => this._reconcileDetach(serial)); + } + + return dheService; + }; + + private _createOrAttachToWorkers = async ( + dheService: IDheService, + workerConsoleType: ConsoleType | undefined = undefined + ): Promise => { + const attachableWorkers = await dheService.listAttachableWorkers( + this._attachedWorkerSerials.keys() + ); + + let workerInfos: [boolean, WorkerInfo][]; + + // If no attachable workers exist, create a new one + if (attachableWorkers.length === 0) { + const workerInfo = await this._createWorker( + dheService, + workerConsoleType + ); + + if (workerInfo == null) { + return []; + } + + workerInfos = [[true, workerInfo]]; + } + // register attachable workers + else { + workerInfos = []; + for (const queryInfo of attachableWorkers) { + try { + workerInfos.push([false, dheService.registerWorkerInfo(queryInfo)]); + } catch (err) { + logger.error( + 'Failed to register attachable worker; skipping:', + queryInfo.serial, + err + ); + } + } + } + + // Connect to workers in parallel + const connections = ( + await Promise.all( + workerInfos.map(([isOwned, workerInfo]) => + this._attachToWorker( + workerInfo.name, + dheService.serverUrl, + isOwned, + workerInfo + ) + ) + ) + ).filter(c => c != null); + + if (attachableWorkers.length > 1) { + this._toaster.info(`Attached to ${connections.length} worker(s).`); + } + + return connections; + }; + + private _createWorker = async ( + dheService: IDheService, + workerConsoleType?: ConsoleType + ): Promise => { + const tagId = uniqueId(); + const placeholderUrl = this.addWorkerPlaceholderConnection( + dheService.label, + dheService.serverUrl, + tagId + ); + + try { + const workerInfo = await dheService.createWorker( + tagId, + workerConsoleType + ); + + // If the worker finished creating but there is no placeholder + // connection, the user cancelled before it was ready. + if (!this._connectionMap.has(placeholderUrl)) { + dheService.deleteWorker(workerInfo.workerUrl); + this._onDidUpdate.fire(); + return null; + } + + this.removeWorkerPlaceholderConnection(placeholderUrl); + return workerInfo; + } catch (err) { + if (err instanceof QueryCreationCancelledError) { + logger.info(err); + const msg = 'Connection cancelled.'; + this._outputChannel.appendLine(msg); + this._toaster.info(msg); + } else { + const msg = + err instanceof QueryStartupFailureError + ? err.message + : 'Failed to create worker.'; + logger.error(err); + this._outputChannel.appendLine(msg); + this._toaster.error(msg); + } + + this.removeWorkerPlaceholderConnection(placeholderUrl); + return null; + } + }; + + /** + * Explicitly create a new worker on a DHE server and attach to it. Unlike + * `connectToServer` (which only auto-creates a worker when none are + * attachable), this always creates a worker — it backs the "+" create-worker + * action on DHE server nodes. + * @param dheServerUrl The DHE server to create the worker on. + * @param workerConsoleType Optional console type for the new worker. + * @returns The new connection state, or `null` if client/worker + * creation/attachment failed. + */ + createWorker = async ( + dheServerUrl: URL, + workerConsoleType?: ConsoleType + ): Promise => { + const dheService = await this._dheServiceCache.get(dheServerUrl); + + // Ensure a live client (the server is already connected when the "+" action + // is visible, but guard anyway — this also covers a stale/expired client). + if (!(await dheService.getClient(true, false))) { + return null; + } + + const workerInfo = await this._createWorker(dheService, workerConsoleType); + if (workerInfo == null) { + return null; + } + + return this._attachToWorker( + workerInfo.name, + dheServerUrl, + true, + workerInfo + ); + }; + + /** + * Attach a single worker when a config event indicates it became attachable. + * Idempotent: no-ops if the serial is already connected. + */ + private _reconcileAttach = async ( + dheServerUrl: URL, + dheService: IDheService, + queryInfo: QueryInfo + ): Promise => { + if (this._attachedWorkerSerials.has(queryInfo.serial as QuerySerial)) { + return; + } + try { + const workerInfo = dheService.registerWorkerInfo(queryInfo); + await this._attachToWorker( + workerInfo.name, + dheServerUrl, + false, + workerInfo + ); + } catch (err) { + logger.error('Failed to attach worker:', queryInfo.serial, err); + } + }; + + /** + * Detach a worker when a config event indicates it was removed or died. + * No-ops if the serial is not tracked. + */ + private _reconcileDetach = async (serial: QuerySerial): Promise => { + const workerUrl = this._attachedWorkerSerials.get(serial); + if (workerUrl == null) { + return; + } + await this.disconnectFromServer(workerUrl); }; /** * Add a placeholder connection to represent a pending DHE Core+ woker creation. + * @param label The connection label to show in the UI for this pending worker. * @param serverUrl The DHE server URL the pending worker is associated with. * @param tagId The tag ID of the worker. * @returns The placeholder URL. */ - addWorkerPlaceholderConnection = (serverUrl: URL, tagId: UniqueID): URL => { + addWorkerPlaceholderConnection = ( + label: string, + serverUrl: URL, + tagId: UniqueID + ): URL => { // simple way to keep placeholder urls unique by just adding a tagId as the pathname const placeholderUrl = new URL(serverUrl); placeholderUrl.pathname = tagId; @@ -382,6 +645,7 @@ export class ServerManager implements IServerManager { this._workerURLToServerURLMap.set(placeholderUrl, serverUrl); this._connectionMap.set(placeholderUrl, { + label, isConnected: false, isRunningCode: false, serverUrl: placeholderUrl, @@ -459,6 +723,15 @@ export class ServerManager implements IServerManager { await dheService.deleteWorker(serverOrWorkerUrl as WorkerURL); } + // Clear the idempotency/teardown gate so a later reconnect can re-attach. + const urlStr = serverOrWorkerUrl.toString(); + for (const [serial, url] of this._attachedWorkerSerials.entries()) { + if (url.toString() === urlStr) { + this._attachedWorkerSerials.delete(serial); + break; + } + } + const connection = this._connectionMap.get(serverOrWorkerUrl); if (connection == null) { return; @@ -592,6 +865,23 @@ export class ServerManager implements IServerManager { }); }; + /** + * Get the parent server for a connection. For a DHE worker, resolves the DHE + * server via the worker→server map; for a DHC connection, the connection's + * `serverUrl` is itself the server URL. Returns `undefined` only when no + * matching server is registered. + * @param connection The connection to get the parent server for. + * @returns The parent server state, or `undefined`. + */ + getServerForConnection = ( + connection: ConnectionState + ): ServerState | undefined => { + const serverUrl = + this._workerURLToServerURLMap.get(connection.serverUrl) ?? + connection.serverUrl; + return this.getServer(serverUrl); + }; + /** * Get all URIs associated with a connection. * @param connection diff --git a/src/types/commonTypes.d.ts b/src/types/commonTypes.d.ts index de54d9051..a5b0dd32d 100644 --- a/src/types/commonTypes.d.ts +++ b/src/types/commonTypes.d.ts @@ -170,6 +170,7 @@ export interface WorkerConfig { export interface ConnectionState { readonly isConnected: boolean; readonly isRunningCode?: boolean; + readonly label: string; readonly serverUrl: URL; readonly tagId?: UniqueID; } @@ -185,6 +186,8 @@ export interface WorkerInfo { grpcUrl: GrpcURL; ideUrl: IdeURL; jsapiUrl: JsapiURL; + /** Persistent query name (`queryInfo.name`), as shown in the Query Monitor. */ + name: string; processInfoId: string | null; serial: QuerySerial; workerName: string | null; diff --git a/src/types/serviceTypes.d.ts b/src/types/serviceTypes.d.ts index 3e7f2890b..4fe0405a7 100644 --- a/src/types/serviceTypes.d.ts +++ b/src/types/serviceTypes.d.ts @@ -1,5 +1,6 @@ import * as vscode from 'vscode'; import type { dh as DhcType } from '@deephaven/jsapi-types'; +import type { QueryInfo } from '@deephaven-enterprise/jsapi-types'; import type { ConsoleType, CoreConnectionConfig, @@ -57,6 +58,7 @@ export interface IConfigService { export interface IDhcService extends IDisposable, ConnectionState { readonly isInitialized: boolean; readonly isConnected: boolean; + readonly isOwned: boolean; isRunningCode: boolean; readonly onDidDisconnect: vscode.Event; @@ -80,7 +82,8 @@ export interface IDhcService extends IDisposable, ConnectionState { } export interface IDheService extends ConnectionState, IDisposable { - readonly onDidWorkerTerminate: vscode.Event; + readonly onWorkerAttachable: vscode.Event; + readonly onWorkerRemoved: vscode.Event; getClient( initializeIfNull: false @@ -92,6 +95,10 @@ export interface IDheService extends ConnectionState, IDisposable { getQuerySerialFromTag(tagId: UniqueID): Promise; getServerFeatures(): DheServerFeatures | undefined; getWorkerInfo: (workerUrl: WorkerURL) => WorkerInfo | undefined; + registerWorkerInfo: (queryInfo: QueryInfo) => WorkerInfo; + listAttachableWorkers: ( + exclude: Iterable + ) => Promise; createWorker: ( tagId: UniqueID, consoleType?: ConsoleType @@ -112,7 +119,7 @@ export type ICoreClientFactory = ( */ export type IDhcServiceFactory = IFactory< IDhcService, - [serverUrl: URL, tagId?: UniqueID] + [label: string, serverUrl: URL, isOwned: boolean, tagId?: UniqueID] >; export type IDheClientFactory = ( serverUrl: URL @@ -176,6 +183,10 @@ export interface IServerManager extends IDisposable { workerConsoleType?: ConsoleType, operateAsAnotherUser?: boolean ) => Promise; + createWorker: ( + dheServerUrl: URL, + workerConsoleType?: ConsoleType + ) => Promise; disconnectEditor: (uri: vscode.Uri) => void; disconnectFromDHEServer: (dheServerUrl: URL) => Promise; disconnectFromServer: (serverUrl: URL) => Promise; @@ -183,9 +194,20 @@ export interface IServerManager extends IDisposable { hasConnectionUris: (connection: ConnectionState) => boolean; + /** Whether a client connection to the given server is currently being established. */ + isServerConnecting: (serverUrl: URL) => boolean; + getConnection: (serverUrl: URL) => ConnectionState | undefined; getConnections: (serverOrWorkerUrl?: URL) => ConnectionState[]; getConnectionUris: (connection: ConnectionState) => vscode.Uri[]; + /** + * Get the parent server for a connection. Resolves the DHE server for a DHE + * worker, or the DHC server for a plain DHC connection. Returns `undefined` + * only when no matching server is registered. + */ + getServerForConnection: ( + connection: ConnectionState + ) => ServerState | undefined; getDheServiceForWorker: (maybeWorkerUrl: URL) => Promise; getEditorConnection: (uri: vscode.Uri) => Promise; getWorkerCredentials: ( diff --git a/src/types/treeViewTypes.d.ts b/src/types/treeViewTypes.d.ts index 7f437f3c4..dd31e1a49 100644 --- a/src/types/treeViewTypes.d.ts +++ b/src/types/treeViewTypes.d.ts @@ -9,12 +9,13 @@ export type ServerGroupState = 'Managed' | 'Running' | 'Stopped'; export type ServerNode = ServerGroupState | ServerState; export interface ServerTreeView extends vscode.TreeView {} -export type ServerConnectionNode = ConnectionState | vscode.Uri; +export type ServerConnectionNode = ServerState | ConnectionState | vscode.Uri; export interface ServerConnectionTreeView extends vscode.TreeView {} export type ServerConnectionPanelNode = + | ServerState | ConnectionState | [URL, VariableDefintion]; diff --git a/src/util/__snapshots__/treeViewUtils.spec.ts.snap b/src/util/__snapshots__/treeViewUtils.spec.ts.snap index 48a720433..41d4b4fb1 100644 --- a/src/util/__snapshots__/treeViewUtils.spec.ts.snap +++ b/src/util/__snapshots__/treeViewUtils.spec.ts.snap @@ -1,50 +1,70 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html +exports[`getConnectionServerTreeItem > should fall back to the url host when there is no label 1`] = ` +{ + "collapsibleState": 2, + "contextValue": "isDHEServerConnectionParent", + "iconPath": ThemeIcon { + "color": undefined, + "id": "vm-connect", + }, + "label": "my-dhe-server:8123", +} +`; + +exports[`getConnectionServerTreeItem > should return a labeled server tree item 1`] = ` +{ + "collapsibleState": 2, + "contextValue": "isDHEServerConnectionParent", + "iconPath": ThemeIcon { + "color": undefined, + "id": "vm-connect", + }, + "label": "My DHE Server", +} +`; + exports[`getPanelConnectionTreeItem > should return panel connection tree item: isConnected:false, isInitialized:false 1`] = ` { "collapsibleState": 2, - "description": undefined, "iconPath": ThemeIcon { "color": undefined, "id": "sync~spin", }, - "label": "localhost:10000", + "label": "Some Worker Label", } `; exports[`getPanelConnectionTreeItem > should return panel connection tree item: isConnected:false, isInitialized:true 1`] = ` { "collapsibleState": 2, - "description": "python", "iconPath": ThemeIcon { "color": undefined, "id": "sync~spin", }, - "label": "localhost:10000", + "label": "Some Worker Label", } `; exports[`getPanelConnectionTreeItem > should return panel connection tree item: isConnected:true, isInitialized:false 1`] = ` { "collapsibleState": 2, - "description": undefined, "iconPath": ThemeIcon { "color": undefined, "id": "vm-connect", }, - "label": "localhost:10000", + "label": "Some Worker Label", } `; exports[`getPanelConnectionTreeItem > should return panel connection tree item: isConnected:true, isInitialized:true 1`] = ` { "collapsibleState": 2, - "description": "python", "iconPath": ThemeIcon { "color": undefined, - "id": "vm-connect", + "id": "dh-python", }, - "label": "localhost:10000", + "label": "Some Worker Label", } `; @@ -312,21 +332,37 @@ exports[`getPanelVariableTreeItem > should return panel variable tree item: type } `; -exports[`getServerContextValue > should return contextValue based on server state: isConnected=false, isManaged=false, isRunning=false 1`] = `"isServerStopped"`; +exports[`getServerContextValue > should return contextValue based on server state: isConnected=false, isConnecting=false, isManaged=false, isRunning=false 1`] = `"isServerStopped"`; + +exports[`getServerContextValue > should return contextValue based on server state: isConnected=false, isConnecting=false, isManaged=false, isRunning=true 1`] = `"isServerRunningDisconnected"`; + +exports[`getServerContextValue > should return contextValue based on server state: isConnected=false, isConnecting=false, isManaged=true, isRunning=false 1`] = `"isManagedServerConnecting"`; + +exports[`getServerContextValue > should return contextValue based on server state: isConnected=false, isConnecting=false, isManaged=true, isRunning=true 1`] = `"isManagedServerDisconnected"`; + +exports[`getServerContextValue > should return contextValue based on server state: isConnected=false, isConnecting=true, isManaged=false, isRunning=false 1`] = `"isServerConnecting"`; -exports[`getServerContextValue > should return contextValue based on server state: isConnected=false, isManaged=false, isRunning=true 1`] = `"isServerRunningDisconnected"`; +exports[`getServerContextValue > should return contextValue based on server state: isConnected=false, isConnecting=true, isManaged=false, isRunning=true 1`] = `"isServerConnecting"`; -exports[`getServerContextValue > should return contextValue based on server state: isConnected=false, isManaged=true, isRunning=false 1`] = `"isManagedServerConnecting"`; +exports[`getServerContextValue > should return contextValue based on server state: isConnected=false, isConnecting=true, isManaged=true, isRunning=false 1`] = `"isServerConnecting"`; -exports[`getServerContextValue > should return contextValue based on server state: isConnected=false, isManaged=true, isRunning=true 1`] = `"isManagedServerDisconnected"`; +exports[`getServerContextValue > should return contextValue based on server state: isConnected=false, isConnecting=true, isManaged=true, isRunning=true 1`] = `"isServerConnecting"`; -exports[`getServerContextValue > should return contextValue based on server state: isConnected=true, isManaged=false, isRunning=false 1`] = `"isServerStopped"`; +exports[`getServerContextValue > should return contextValue based on server state: isConnected=true, isConnecting=false, isManaged=false, isRunning=false 1`] = `"isServerStopped"`; -exports[`getServerContextValue > should return contextValue based on server state: isConnected=true, isManaged=false, isRunning=true 1`] = `"isServerRunningConnected"`; +exports[`getServerContextValue > should return contextValue based on server state: isConnected=true, isConnecting=false, isManaged=false, isRunning=true 1`] = `"isServerRunningConnected"`; -exports[`getServerContextValue > should return contextValue based on server state: isConnected=true, isManaged=true, isRunning=false 1`] = `"isManagedServerConnected"`; +exports[`getServerContextValue > should return contextValue based on server state: isConnected=true, isConnecting=false, isManaged=true, isRunning=false 1`] = `"isManagedServerConnected"`; -exports[`getServerContextValue > should return contextValue based on server state: isConnected=true, isManaged=true, isRunning=true 1`] = `"isManagedServerConnected"`; +exports[`getServerContextValue > should return contextValue based on server state: isConnected=true, isConnecting=false, isManaged=true, isRunning=true 1`] = `"isManagedServerConnected"`; + +exports[`getServerContextValue > should return contextValue based on server state: isConnected=true, isConnecting=true, isManaged=false, isRunning=false 1`] = `"isServerConnecting"`; + +exports[`getServerContextValue > should return contextValue based on server state: isConnected=true, isConnecting=true, isManaged=false, isRunning=true 1`] = `"isServerConnecting"`; + +exports[`getServerContextValue > should return contextValue based on server state: isConnected=true, isConnecting=true, isManaged=true, isRunning=false 1`] = `"isServerConnecting"`; + +exports[`getServerContextValue > should return contextValue based on server state: isConnected=true, isConnecting=true, isManaged=true, isRunning=true 1`] = `"isServerConnecting"`; exports[`getServerDescription > should return server description based on parameters: connectionCount=0, isManaged=false, label=some label 1`] = `"some label"`; @@ -400,23 +436,39 @@ exports[`getServerGroupTreeItem > should return server group tree item: group=Ru } `; -exports[`getServerIconID > should return icon id based on server state: isConnected=false, isManaged=false, isRunning=false 1`] = `"circle-slash"`; +exports[`getServerIconID > should return icon id based on server state: isConnected=false, isConnecting=false, isManaged=false, isRunning=false 1`] = `"circle-slash"`; + +exports[`getServerIconID > should return icon id based on server state: isConnected=false, isConnecting=false, isManaged=false, isRunning=true 1`] = `"circle-large-outline"`; + +exports[`getServerIconID > should return icon id based on server state: isConnected=false, isConnecting=false, isManaged=true, isRunning=false 1`] = `"sync~spin"`; + +exports[`getServerIconID > should return icon id based on server state: isConnected=false, isConnecting=false, isManaged=true, isRunning=true 1`] = `"circle-large-outline"`; -exports[`getServerIconID > should return icon id based on server state: isConnected=false, isManaged=false, isRunning=true 1`] = `"circle-large-outline"`; +exports[`getServerIconID > should return icon id based on server state: isConnected=false, isConnecting=true, isManaged=false, isRunning=false 1`] = `"sync~spin"`; -exports[`getServerIconID > should return icon id based on server state: isConnected=false, isManaged=true, isRunning=false 1`] = `"sync~spin"`; +exports[`getServerIconID > should return icon id based on server state: isConnected=false, isConnecting=true, isManaged=false, isRunning=true 1`] = `"sync~spin"`; -exports[`getServerIconID > should return icon id based on server state: isConnected=false, isManaged=true, isRunning=true 1`] = `"circle-large-outline"`; +exports[`getServerIconID > should return icon id based on server state: isConnected=false, isConnecting=true, isManaged=true, isRunning=false 1`] = `"sync~spin"`; -exports[`getServerIconID > should return icon id based on server state: isConnected=true, isManaged=false, isRunning=false 1`] = `"circle-slash"`; +exports[`getServerIconID > should return icon id based on server state: isConnected=false, isConnecting=true, isManaged=true, isRunning=true 1`] = `"sync~spin"`; -exports[`getServerIconID > should return icon id based on server state: isConnected=true, isManaged=false, isRunning=true 1`] = `"circle-large-filled"`; +exports[`getServerIconID > should return icon id based on server state: isConnected=true, isConnecting=false, isManaged=false, isRunning=false 1`] = `"circle-slash"`; -exports[`getServerIconID > should return icon id based on server state: isConnected=true, isManaged=true, isRunning=false 1`] = `"sync~spin"`; +exports[`getServerIconID > should return icon id based on server state: isConnected=true, isConnecting=false, isManaged=false, isRunning=true 1`] = `"circle-large-filled"`; -exports[`getServerIconID > should return icon id based on server state: isConnected=true, isManaged=true, isRunning=true 1`] = `"circle-large-filled"`; +exports[`getServerIconID > should return icon id based on server state: isConnected=true, isConnecting=false, isManaged=true, isRunning=false 1`] = `"sync~spin"`; -exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=false, isManaged=false, isRunning=false 1`] = ` +exports[`getServerIconID > should return icon id based on server state: isConnected=true, isConnecting=false, isManaged=true, isRunning=true 1`] = `"circle-large-filled"`; + +exports[`getServerIconID > should return icon id based on server state: isConnected=true, isConnecting=true, isManaged=false, isRunning=false 1`] = `"sync~spin"`; + +exports[`getServerIconID > should return icon id based on server state: isConnected=true, isConnecting=true, isManaged=false, isRunning=true 1`] = `"sync~spin"`; + +exports[`getServerIconID > should return icon id based on server state: isConnected=true, isConnecting=true, isManaged=true, isRunning=false 1`] = `"sync~spin"`; + +exports[`getServerIconID > should return icon id based on server state: isConnected=true, isConnecting=true, isManaged=true, isRunning=true 1`] = `"sync~spin"`; + +exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=false, isConnecting=false, isManaged=false, isRunning=false 1`] = ` { "command": undefined, "contextValue": "isServerStopped", @@ -430,7 +482,7 @@ exports[`getServerTreeItem > should return server tree item: type=DHC, isConnect } `; -exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=false, isManaged=false, isRunning=true 1`] = ` +exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=false, isConnecting=false, isManaged=false, isRunning=true 1`] = ` { "command": { "arguments": [ @@ -457,7 +509,7 @@ exports[`getServerTreeItem > should return server tree item: type=DHC, isConnect } `; -exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=false, isManaged=true, isRunning=false 1`] = ` +exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=false, isConnecting=false, isManaged=true, isRunning=false 1`] = ` { "command": undefined, "contextValue": "isManagedServerConnecting", @@ -471,7 +523,7 @@ exports[`getServerTreeItem > should return server tree item: type=DHC, isConnect } `; -exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=false, isManaged=true, isRunning=true 1`] = ` +exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=false, isConnecting=false, isManaged=true, isRunning=true 1`] = ` { "command": { "arguments": [ @@ -499,7 +551,63 @@ exports[`getServerTreeItem > should return server tree item: type=DHC, isConnect } `; -exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=true, isManaged=false, isRunning=false 1`] = ` +exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=false, isConnecting=true, isManaged=false, isRunning=false 1`] = ` +{ + "command": undefined, + "contextValue": "isServerConnecting", + "description": "", + "iconPath": ThemeIcon { + "color": undefined, + "id": "sync~spin", + }, + "label": "localhost:10000", + "tooltip": "Connecting to localhost:10000…", +} +`; + +exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=false, isConnecting=true, isManaged=false, isRunning=true 1`] = ` +{ + "command": undefined, + "contextValue": "isServerConnecting", + "description": "", + "iconPath": ThemeIcon { + "color": undefined, + "id": "sync~spin", + }, + "label": "localhost:10000", + "tooltip": "Connecting to localhost:10000…", +} +`; + +exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=false, isConnecting=true, isManaged=true, isRunning=false 1`] = ` +{ + "command": undefined, + "contextValue": "isServerConnecting", + "description": "pip", + "iconPath": ThemeIcon { + "color": undefined, + "id": "sync~spin", + }, + "label": "localhost:10000", + "tooltip": "Connecting to localhost:10000…", +} +`; + +exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=false, isConnecting=true, isManaged=true, isRunning=true 1`] = ` +{ + "command": undefined, + "contextValue": "isServerConnecting", + "description": "pip", + "iconPath": ThemeIcon { + "color": undefined, + "id": "sync~spin", + }, + "label": "localhost:10000", + "tooltip": "Connecting to localhost:10000…", +} +`; + +exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=true, isConnecting=false, isManaged=false, isRunning=false 1`] = ` { "command": undefined, "contextValue": "isServerStopped", @@ -513,7 +621,7 @@ exports[`getServerTreeItem > should return server tree item: type=DHC, isConnect } `; -exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=true, isManaged=false, isRunning=true 1`] = ` +exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=true, isConnecting=false, isManaged=false, isRunning=true 1`] = ` { "command": undefined, "contextValue": "isServerRunningConnected", @@ -527,7 +635,7 @@ exports[`getServerTreeItem > should return server tree item: type=DHC, isConnect } `; -exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=true, isManaged=true, isRunning=false 1`] = ` +exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=true, isConnecting=false, isManaged=true, isRunning=false 1`] = ` { "command": undefined, "contextValue": "isManagedServerConnected", @@ -541,7 +649,7 @@ exports[`getServerTreeItem > should return server tree item: type=DHC, isConnect } `; -exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=true, isManaged=true, isRunning=true 1`] = ` +exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=true, isConnecting=false, isManaged=true, isRunning=true 1`] = ` { "command": undefined, "contextValue": "isManagedServerConnected", @@ -555,7 +663,63 @@ exports[`getServerTreeItem > should return server tree item: type=DHC, isConnect } `; -exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=false, isManaged=false, isRunning=false 1`] = ` +exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=true, isConnecting=true, isManaged=false, isRunning=false 1`] = ` +{ + "command": undefined, + "contextValue": "isServerConnecting", + "description": "(1)", + "iconPath": ThemeIcon { + "color": undefined, + "id": "sync~spin", + }, + "label": "localhost:10000", + "tooltip": "Connecting to localhost:10000…", +} +`; + +exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=true, isConnecting=true, isManaged=false, isRunning=true 1`] = ` +{ + "command": undefined, + "contextValue": "isServerConnecting", + "description": "(1)", + "iconPath": ThemeIcon { + "color": undefined, + "id": "sync~spin", + }, + "label": "localhost:10000", + "tooltip": "Connecting to localhost:10000…", +} +`; + +exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=true, isConnecting=true, isManaged=true, isRunning=false 1`] = ` +{ + "command": undefined, + "contextValue": "isServerConnecting", + "description": "pip (1)", + "iconPath": ThemeIcon { + "color": undefined, + "id": "sync~spin", + }, + "label": "localhost:10000", + "tooltip": "Connecting to localhost:10000…", +} +`; + +exports[`getServerTreeItem > should return server tree item: type=DHC, isConnected=true, isConnecting=true, isManaged=true, isRunning=true 1`] = ` +{ + "command": undefined, + "contextValue": "isServerConnecting", + "description": "pip (1)", + "iconPath": ThemeIcon { + "color": undefined, + "id": "sync~spin", + }, + "label": "localhost:10000", + "tooltip": "Connecting to localhost:10000…", +} +`; + +exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=false, isConnecting=false, isManaged=false, isRunning=false 1`] = ` { "command": undefined, "contextValue": "isServerStopped", @@ -569,7 +733,7 @@ exports[`getServerTreeItem > should return server tree item: type=DHE, isConnect } `; -exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=false, isManaged=false, isRunning=true 1`] = ` +exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=false, isConnecting=false, isManaged=false, isRunning=true 1`] = ` { "command": { "arguments": [ @@ -596,7 +760,7 @@ exports[`getServerTreeItem > should return server tree item: type=DHE, isConnect } `; -exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=false, isManaged=true, isRunning=false 1`] = ` +exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=false, isConnecting=false, isManaged=true, isRunning=false 1`] = ` { "command": undefined, "contextValue": "isManagedServerConnecting", @@ -610,7 +774,7 @@ exports[`getServerTreeItem > should return server tree item: type=DHE, isConnect } `; -exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=false, isManaged=true, isRunning=true 1`] = ` +exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=false, isConnecting=false, isManaged=true, isRunning=true 1`] = ` { "command": { "arguments": [ @@ -638,7 +802,63 @@ exports[`getServerTreeItem > should return server tree item: type=DHE, isConnect } `; -exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=true, isManaged=false, isRunning=false 1`] = ` +exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=false, isConnecting=true, isManaged=false, isRunning=false 1`] = ` +{ + "command": undefined, + "contextValue": "isServerConnecting", + "description": "", + "iconPath": ThemeIcon { + "color": undefined, + "id": "sync~spin", + }, + "label": "localhost:10000", + "tooltip": "Connecting to localhost:10000…", +} +`; + +exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=false, isConnecting=true, isManaged=false, isRunning=true 1`] = ` +{ + "command": undefined, + "contextValue": "isServerConnecting", + "description": "", + "iconPath": ThemeIcon { + "color": undefined, + "id": "sync~spin", + }, + "label": "localhost:10000", + "tooltip": "Connecting to localhost:10000…", +} +`; + +exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=false, isConnecting=true, isManaged=true, isRunning=false 1`] = ` +{ + "command": undefined, + "contextValue": "isServerConnecting", + "description": "pip", + "iconPath": ThemeIcon { + "color": undefined, + "id": "sync~spin", + }, + "label": "localhost:10000", + "tooltip": "Connecting to localhost:10000…", +} +`; + +exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=false, isConnecting=true, isManaged=true, isRunning=true 1`] = ` +{ + "command": undefined, + "contextValue": "isServerConnecting", + "description": "pip", + "iconPath": ThemeIcon { + "color": undefined, + "id": "sync~spin", + }, + "label": "localhost:10000", + "tooltip": "Connecting to localhost:10000…", +} +`; + +exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=true, isConnecting=false, isManaged=false, isRunning=false 1`] = ` { "command": undefined, "contextValue": "isServerStopped", @@ -652,7 +872,7 @@ exports[`getServerTreeItem > should return server tree item: type=DHE, isConnect } `; -exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=true, isManaged=false, isRunning=true 1`] = ` +exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=true, isConnecting=false, isManaged=false, isRunning=true 1`] = ` { "command": { "arguments": [ @@ -679,7 +899,7 @@ exports[`getServerTreeItem > should return server tree item: type=DHE, isConnect } `; -exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=true, isManaged=true, isRunning=false 1`] = ` +exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=true, isConnecting=false, isManaged=true, isRunning=false 1`] = ` { "command": undefined, "contextValue": "isManagedServerConnected", @@ -693,7 +913,7 @@ exports[`getServerTreeItem > should return server tree item: type=DHE, isConnect } `; -exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=true, isManaged=true, isRunning=true 1`] = ` +exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=true, isConnecting=false, isManaged=true, isRunning=true 1`] = ` { "command": undefined, "contextValue": "isManagedServerConnected", @@ -707,6 +927,62 @@ exports[`getServerTreeItem > should return server tree item: type=DHE, isConnect } `; +exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=true, isConnecting=true, isManaged=false, isRunning=false 1`] = ` +{ + "command": undefined, + "contextValue": "isServerConnecting", + "description": "(1)", + "iconPath": ThemeIcon { + "color": undefined, + "id": "sync~spin", + }, + "label": "localhost:10000", + "tooltip": "Connecting to localhost:10000…", +} +`; + +exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=true, isConnecting=true, isManaged=false, isRunning=true 1`] = ` +{ + "command": undefined, + "contextValue": "isServerConnecting", + "description": "(1)", + "iconPath": ThemeIcon { + "color": undefined, + "id": "sync~spin", + }, + "label": "localhost:10000", + "tooltip": "Connecting to localhost:10000…", +} +`; + +exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=true, isConnecting=true, isManaged=true, isRunning=false 1`] = ` +{ + "command": undefined, + "contextValue": "isServerConnecting", + "description": "pip (1)", + "iconPath": ThemeIcon { + "color": undefined, + "id": "sync~spin", + }, + "label": "localhost:10000", + "tooltip": "Connecting to localhost:10000…", +} +`; + +exports[`getServerTreeItem > should return server tree item: type=DHE, isConnected=true, isConnecting=true, isManaged=true, isRunning=true 1`] = ` +{ + "command": undefined, + "contextValue": "isServerConnecting", + "description": "pip (1)", + "iconPath": ThemeIcon { + "color": undefined, + "id": "sync~spin", + }, + "label": "localhost:10000", + "tooltip": "Connecting to localhost:10000…", +} +`; + exports[`getVariableIconPath > should return icon path for variableType 1`] = ` [ [ diff --git a/src/util/__snapshots__/uiUtils.spec.ts.snap b/src/util/__snapshots__/uiUtils.spec.ts.snap index 9c7e875cf..7c067cc81 100644 --- a/src/util/__snapshots__/uiUtils.spec.ts.snap +++ b/src/util/__snapshots__/uiUtils.spec.ts.snap @@ -31,27 +31,29 @@ exports[`createConnectionQuickPickOptions > should return quick pick options: ed { "data": { "isConnected": true, + "label": "ServerA Connection", "serverUrl": "http://localhost:10000/", }, - "description": "python (current)", + "description": "localhost:10000 (current)", "iconPath": ThemeIcon { "color": undefined, - "id": "vm-connect", + "id": "dh-python", }, - "label": "http://localhost:10000/", + "label": "ServerA Connection", "type": "connection", }, { "data": { "isConnected": true, + "label": "ServerC Connection", "serverUrl": "http://localhost:10002/", }, - "description": "python", + "description": "localhost:10002", "iconPath": ThemeIcon { "color": undefined, - "id": "vm-connect", + "id": "dh-python", }, - "label": "http://localhost:10002/", + "label": "ServerC Connection", "type": "connection", }, { @@ -102,27 +104,29 @@ exports[`createConnectionQuickPickOptions > should return quick pick options: ed { "data": { "isConnected": true, + "label": "ServerA Connection", "serverUrl": "http://localhost:10000/", }, - "description": "python", + "description": "localhost:10000", "iconPath": ThemeIcon { "color": undefined, - "id": "vm-connect", + "id": "dh-python", }, - "label": "http://localhost:10000/", + "label": "ServerA Connection", "type": "connection", }, { "data": { "isConnected": true, + "label": "ServerC Connection", "serverUrl": "http://localhost:10002/", }, - "description": "python", + "description": "localhost:10002", "iconPath": ThemeIcon { "color": undefined, - "id": "vm-connect", + "id": "dh-python", }, - "label": "http://localhost:10002/", + "label": "ServerC Connection", "type": "connection", }, { diff --git a/src/util/treeViewUtils.spec.ts b/src/util/treeViewUtils.spec.ts index 7630e67e7..b294c0fc9 100644 --- a/src/util/treeViewUtils.spec.ts +++ b/src/util/treeViewUtils.spec.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi } from 'vitest'; import { bitValues, boolValues, matrix } from '../testUtils'; import { + getConnectionServerTreeItem, getPanelConnectionTreeItem, getPanelVariableTreeItem, getServerContextValue, @@ -59,15 +60,46 @@ describe('getPanelConnectionTreeItem', () => { vi.mocked(isInstanceOf).mockReturnValue(true); - const actual = await getPanelConnectionTreeItem(connection, async () => { - const [consoleType] = await getConsoleTypes(); - return isInitialized ? consoleType : undefined; - }); + const actual = await getPanelConnectionTreeItem( + connection, + async () => { + const [consoleType] = await getConsoleTypes(); + return isInitialized ? consoleType : undefined; + }, + 'Some Worker Label' + ); expect(actual).toMatchSnapshot(); } ); }); +describe('getConnectionServerTreeItem', () => { + it('should return a labeled server tree item', () => { + const server: ServerState = { + type: 'DHE', + url: new URL('https://my-dhe-server:8123'), + label: 'My DHE Server', + isConnected: true, + isRunning: true, + connectionCount: 2, + }; + + expect(getConnectionServerTreeItem(server)).toMatchSnapshot(); + }); + + it('should fall back to the url host when there is no label', () => { + const server: ServerState = { + type: 'DHE', + url: new URL('https://my-dhe-server:8123'), + isConnected: true, + isRunning: true, + connectionCount: 2, + }; + + expect(getConnectionServerTreeItem(server)).toMatchSnapshot(); + }); +}); + describe('getPanelVariableTreeItem', () => { const url = new URL('http://localhost:10000'); @@ -86,11 +118,12 @@ describe('getPanelVariableTreeItem', () => { }); describe('getServerContextValue', () => { - it.each(matrix(boolValues, boolValues, boolValues))( - 'should return contextValue based on server state: isConnected=%s, isManaged=%s, isRunning=%s', - (isConnected, isManaged, isRunning) => { + it.each(matrix(boolValues, boolValues, boolValues, boolValues))( + 'should return contextValue based on server state: isConnected=%s, isConnecting=%s, isManaged=%s, isRunning=%s', + (isConnected, isConnecting, isManaged, isRunning) => { const actual = getServerContextValue({ isConnected, + isConnecting, isDHE: false, isManaged, isRunning, @@ -137,10 +170,15 @@ describe('getServerGroupTreeItem', () => { }); describe('getServerIconID', () => { - it.each(matrix(boolValues, boolValues, boolValues))( - 'should return icon id based on server state: isConnected=%s, isManaged=%s, isRunning=%s', - (isConnected, isManaged, isRunning) => { - const actual = getServerIconID({ isConnected, isManaged, isRunning }); + it.each(matrix(boolValues, boolValues, boolValues, boolValues))( + 'should return icon id based on server state: isConnected=%s, isConnecting=%s, isManaged=%s, isRunning=%s', + (isConnected, isConnecting, isManaged, isRunning) => { + const actual = getServerIconID({ + isConnected, + isConnecting, + isManaged, + isRunning, + }); expect(actual).toMatchSnapshot(); } ); @@ -157,19 +195,22 @@ describe('getServerTreeItem', () => { connectionCount: 0, }; - it.each(matrix(typeValues, boolValues, boolValues, boolValues))( - 'should return server tree item: type=%s, isConnected=%s, isManaged=%s, isRunning=%s', - (type, isConnected, isManaged, isRunning) => { - const actual = getServerTreeItem({ - ...dhcServerState, - ...(isManaged - ? { isManaged: true, psk: 'mock.psk' as Psk } - : { isManaged: false }), - type, - connectionCount: isConnected ? 1 : 0, - isConnected, - isRunning, - }); + it.each(matrix(typeValues, boolValues, boolValues, boolValues, boolValues))( + 'should return server tree item: type=%s, isConnected=%s, isConnecting=%s, isManaged=%s, isRunning=%s', + (type, isConnected, isConnecting, isManaged, isRunning) => { + const actual = getServerTreeItem( + { + ...dhcServerState, + ...(isManaged + ? { isManaged: true, psk: 'mock.psk' as Psk } + : { isManaged: false }), + type, + connectionCount: isConnected ? 1 : 0, + isConnected, + isRunning, + }, + isConnecting + ); expect(actual).toMatchSnapshot(); } diff --git a/src/util/treeViewUtils.ts b/src/util/treeViewUtils.ts index 686ed10c9..36dffc4b8 100644 --- a/src/util/treeViewUtils.ts +++ b/src/util/treeViewUtils.ts @@ -2,6 +2,7 @@ import * as vscode from 'vscode'; import type { ConnectionState, ConsoleType, + IServerManager, NonEmptyArray, ServerGroupState, ServerState, @@ -9,6 +10,7 @@ import type { VariableType, } from '../types'; import { + CONNECTION_TREE_ITEM_CONTEXT, DH_PROTECTED_VARIABLE_NAMES, ICON_ID, OPEN_VARIABLE_PANELS_CMD, @@ -48,6 +50,26 @@ export function getVariableIconPath( } } +/** + * Get the icon id for a console type / language, used for connection tree nodes. + * Falls back to the generic "connected" icon when the console type is unknown + * (e.g. a plain DHC connection or one whose console type has not resolved yet). + * @param consoleType Console type (language) of the connection, if known. + * @returns Icon id from `ICON_ID`. + */ +export function getConsoleTypeIconId( + consoleType: ConsoleType | undefined +): string { + switch (consoleType) { + case 'python': + return ICON_ID.python; + case 'groovy': + return ICON_ID.groovy; + default: + return ICON_ID.connected; + } +} + /** * Get `TreeItem` for a panel connection. * @param connection Connection state @@ -58,30 +80,20 @@ export async function getPanelConnectionTreeItem( getConsoleType: ( connection: ConnectionState ) => Promise, - serverLabel?: string + label: string ): Promise { - const descriptionTokens: string[] = []; - + // Console type (language) drives the node icon rather than the description. const consoleType = await getConsoleType(connection); - if (consoleType) { - descriptionTokens.push(consoleType); - } - - if (connection.tagId) { - descriptionTokens.push(connection.tagId); - } - - const label = serverLabel ?? connection.serverUrl.host; - const description = - descriptionTokens.length === 0 ? undefined : descriptionTokens.join(' - '); - return { label, - description, collapsibleState: vscode.TreeItemCollapsibleState.Expanded, + // Show the language (Python/Groovy) icon when idle/connected; show the + // spinner while busy (connecting or running code). iconPath: new vscode.ThemeIcon( - connection.isConnected ? ICON_ID.connected : ICON_ID.connecting + connection.isRunningCode || !connection.isConnected + ? ICON_ID.connecting + : getConsoleTypeIconId(consoleType) ), }; } @@ -111,24 +123,111 @@ export function getPanelVariableTreeItem([url, variable]: [ }; } +/** + * Type guard for a (DHE) server node within the connection / panel tree root. + * `ServerState` carries `url`; `ConnectionState` carries `serverUrl`. + * @param node A server or connection root node. + */ +export function isServerStateNode( + node: ServerState | ConnectionState +): node is ServerState { + return 'url' in node; +} + +/** + * Get the label shown for a server node in the connection / panel tree views. + * @param server Server state. + */ +export function getConnectionServerLabel(server: ServerState): string { + return server.label ?? server.url.host; +} + +/** + * Compute the root nodes for the connection / panel tree views. Every + * connection is grouped under its parent server node (DHC and DHE alike), so a + * community server with a single worker has the same hierarchy shape as an + * enterprise server with many. Roots are sorted by their displayed label. + * + * In addition to every server that has connections, any DHE server with a live + * client is included so its node (and its "+" create-worker action) stays + * reachable even with zero workers. + * @param serverManager Server manager. + * @returns The server root nodes (servers with connections, plus connected DHE + * servers). + */ +export function getConnectionTreeRootNodes( + serverManager: IServerManager +): ServerState[] { + const servers = new Map(); + + for (const connection of serverManager.getConnections()) { + const server = serverManager.getServerForConnection(connection); + if (server != null) { + servers.set(server.url.toString(), server); + } + } + + // DHE servers with a live client appear even with zero workers, so the server + // node (and its "+" create-worker action) stays reachable after a cancelled + // worker creation or after the last worker is detached. + for (const server of serverManager.getServers({ type: 'DHE' })) { + if (server.isConnected) { + servers.set(server.url.toString(), server); + } + } + + return [...servers.values()].sort((a, b) => + getConnectionServerLabel(a).localeCompare(getConnectionServerLabel(b)) + ); +} + +/** + * Get `TreeItem` for a DHE server node in the connection / panel tree views. + * This is a grouping container whose children are the server's worker + * connections. + * @param server DHE server state + */ +export function getConnectionServerTreeItem( + server: ServerState +): vscode.TreeItem { + return { + label: getConnectionServerLabel(server), + // The "computer" icon (`vm-connect`) previously used for worker connection + // nodes, before the language (Python/Groovy) icons took their place. + iconPath: new vscode.ThemeIcon(ICON_ID.connected), + collapsibleState: vscode.TreeItemCollapsibleState.Expanded, + contextValue: + server.type === 'DHE' + ? CONNECTION_TREE_ITEM_CONTEXT.isDHEServerConnectionParent + : undefined, + }; +} + /** * Get `contextValue` for server tree items. * @param isConnected Whether the server is connected + * @param isConnecting Whether a client connection is currently being established * @param isDHE Whether the server is a DHE server * @param isManaged Whether the server is managed * @param isRunning Whether the server is running */ export function getServerContextValue({ isConnected, + isConnecting, isDHE, isManaged, isRunning, }: { isConnected: boolean; + isConnecting: boolean; isDHE: boolean; isManaged: boolean; isRunning: boolean; }): ServerTreeItemContextValue { + if (isConnecting) { + return SERVER_TREE_ITEM_CONTEXT.isServerConnecting; + } + if (isManaged) { return isConnected ? SERVER_TREE_ITEM_CONTEXT.isManagedServerConnected @@ -217,19 +316,26 @@ export function getServerGroupTreeItem( /** * Get icon id for a server in the UI. e.g. for tree nodes. * @param isConnected Whether the server is connected + * @param isConnecting Whether a client connection is currently being established * @param isManaged Whether the server is managed * @param isRunning Whether the server is running * @returns Icon id for server tree item */ export function getServerIconID({ isConnected, + isConnecting, isManaged, isRunning, }: { isConnected: boolean; + isConnecting: boolean; isManaged: boolean; isRunning: boolean; }): string { + if (isConnecting) { + return ICON_ID.connecting; + } + return isRunning ? isConnected ? ICON_ID.serverConnected @@ -246,9 +352,13 @@ export function getServerIconID({ * number of connected workers in the case of DHE) * @param isManaged Whether the server is managed * @param isRunning Whether the server is running + * @param isConnecting Whether a client connection is currently being established * @returns Tree item representing the server */ -export function getServerTreeItem(server: ServerState): vscode.TreeItem { +export function getServerTreeItem( + server: ServerState, + isConnecting: boolean +): vscode.TreeItem { const { connectionCount, isConnected, @@ -259,6 +369,7 @@ export function getServerTreeItem(server: ServerState): vscode.TreeItem { const contextValue = getServerContextValue({ isConnected, + isConnecting, isDHE: type === 'DHE', isManaged, isRunning, @@ -266,24 +377,25 @@ export function getServerTreeItem(server: ServerState): vscode.TreeItem { const description = getServerDescription(connectionCount, isManaged); - const urlStr = server.url.toString(); - const canConnect = contextValue === SERVER_TREE_ITEM_CONTEXT.isManagedServerDisconnected || contextValue === SERVER_TREE_ITEM_CONTEXT.isServerRunningDisconnected || contextValue === SERVER_TREE_ITEM_CONTEXT.isDHEServerRunningConnected || contextValue === SERVER_TREE_ITEM_CONTEXT.isDHEServerRunningDisconnected; - const url = new URL(urlStr); - const label = server.label ?? url.host; + const label = getConnectionServerLabel(server); return { label, description, - tooltip: canConnect ? `Click to connect to ${label}` : label, + tooltip: isConnecting + ? `Connecting to ${label}…` + : canConnect + ? `Click to connect to ${label}` + : label, contextValue, iconPath: new vscode.ThemeIcon( - getServerIconID({ isConnected, isManaged, isRunning }) + getServerIconID({ isConnected, isConnecting, isManaged, isRunning }) ), command: canConnect ? { diff --git a/src/util/uiUtils.spec.ts b/src/util/uiUtils.spec.ts index c78b4fb54..24e61b8a4 100644 --- a/src/util/uiUtils.spec.ts +++ b/src/util/uiUtils.spec.ts @@ -14,6 +14,7 @@ import type { ConnectionState, CoreConnectionConfig, IDhcService, + IServerManager, ServerState, } from '../types'; @@ -55,7 +56,7 @@ describe('createConnectionQuickPickOptions', () => { ['Active A', serverUrlA], ])( 'should return quick pick options: editorActiveConnectionUrl=%s', - (_label, editorActiveConnectionUrl) => { + async (_label, editorActiveConnectionUrl) => { const serversWithoutConnections: ServerState[] = [ { type: 'DHC', @@ -73,27 +74,59 @@ describe('createConnectionQuickPickOptions', () => { }, ]; const connections: ConnectionState[] = [ - { serverUrl: serverUrlA, isConnected: true }, - { serverUrl: serverUrlC, isConnected: true }, + { + label: 'ServerA Connection', + serverUrl: serverUrlA, + isConnected: true, + }, + { + label: 'ServerC Connection', + serverUrl: serverUrlC, + isConnected: true, + }, ]; - const actual = createConnectionQuickPickOptions( + const serverManager = { + getServerForConnection: vi.fn( + (connection: ConnectionState): ServerState => ({ + type: 'DHC', + url: connection.serverUrl, + isConnected: true, + isRunning: true, + connectionCount: 1, + }) + ), + getWorkerInfo: vi.fn(async () => undefined), + } as unknown as IServerManager; + + const actual = await createConnectionQuickPickOptions( serversWithoutConnections, connections, 'python', + serverManager, editorActiveConnectionUrl ); expect(actual).toMatchSnapshot(); } ); - it('should throw if no servers or connections', () => { + it('should throw if no servers or connections', async () => { const servers: ServerState[] = []; const connections: IDhcService[] = []; - expect(() => - createConnectionQuickPickOptions(servers, connections, 'python') - ).toThrowError('No available servers to connect to.'); + const serverManager = { + getServerForConnection: vi.fn(), + getWorkerInfo: vi.fn(async () => undefined), + } as unknown as IServerManager; + + await expect( + createConnectionQuickPickOptions( + servers, + connections, + 'python', + serverManager + ) + ).rejects.toThrowError('No available servers to connect to.'); }); }); diff --git a/src/util/uiUtils.ts b/src/util/uiUtils.ts index 53ceaa19f..50ac53373 100644 --- a/src/util/uiUtils.ts +++ b/src/util/uiUtils.ts @@ -19,6 +19,7 @@ import type { ConnectionType, ConsoleType, ConnectionPickItem, + IServerManager, ServerState, SeparatorPickItem, ConnectionPickOption, @@ -32,6 +33,7 @@ import type { MultiAuthConfig, } from '../types'; import { getFilePathDateToken, sortByStringProp } from './dataUtils'; +import { getConsoleTypeIconId } from './treeViewUtils'; import { Logger } from './Logger'; const logger = new Logger('uiUtils'); @@ -54,45 +56,63 @@ export interface WorkspaceFolderConfig { /** * Create options for a connection quick pick. + * + * Active-connection items mirror the WORKERS-tree worker node: a leading + * language icon plus the worker name, with the server host:port shown in the + * description. * @param servers The available servers * @param connections The available connections * @param editorLanguageId The language id of the editor + * @param serverManager The server manager used to resolve each connection's + * parent server and worker info. * @param editorActiveConnectionUrl The active connection url of the editor * @returns */ -export function createConnectionQuickPickOptions< +export async function createConnectionQuickPickOptions< TConnection extends ConnectionState, >( servers: ServerState[], connections: TConnection[], editorLanguageId: string, + serverManager: IServerManager, editorActiveConnectionUrl?: URL | null -): ConnectionPickOption[] { +): Promise[]> { const serverOptions: ConnectionPickItem<'server', ServerState>[] = servers.map(data => ({ type: 'server', - label: data.url.toString(), - description: data.label ?? (data.isManaged ? 'pip' : undefined), + label: data.label ?? data.url.toString(), + description: data.isManaged ? 'pip' : undefined, iconPath: new vscode.ThemeIcon(ICON_ID.server), data, })); - const connectionOptions: ConnectionPickItem<'connection', TConnection>[] = []; - - for (const dhService of connections) { - const isActiveConnection = - editorActiveConnectionUrl?.toString() === dhService.serverUrl.toString(); - - connectionOptions.push({ - type: 'connection', - label: dhService.serverUrl.toString(), - iconPath: new vscode.ThemeIcon(ICON_ID.connected), - description: isActiveConnection - ? `${editorLanguageId} (current)` - : editorLanguageId, - data: dhService, - }); - } + const connectionOptions: ConnectionPickItem<'connection', TConnection>[] = + await Promise.all( + connections.map(async dhService => { + const isActiveConnection = + editorActiveConnectionUrl?.toString() === + dhService.serverUrl.toString(); + + const parentServer = serverManager.getServerForConnection(dhService); + assertDefined(parentServer, 'parentServer'); + + const descriptionTokens = [parentServer.label ?? parentServer.url.host]; + + if (isActiveConnection) { + descriptionTokens.push('(current)'); + } + + return { + type: 'connection' as const, + label: dhService.label, + iconPath: new vscode.ThemeIcon( + getConsoleTypeIconId(editorLanguageId as ConsoleType) + ), + description: descriptionTokens.join(' '), + data: dhService, + }; + }) + ); if (serverOptions.length === 0 && connectionOptions.length === 0) { throw new Error('No available servers to connect to.');