Multi-connection playgrounds with per-document isolation#583
Merged
tnaum-ms merged 24 commits intorel/release_0.8.0from Apr 15, 2026
Merged
Multi-connection playgrounds with per-document isolation#583tnaum-ms merged 24 commits intorel/release_0.8.0from
tnaum-ms merged 24 commits intorel/release_0.8.0from
Conversation
Each playground document is permanently bound to its cluster/database. Multiple playgrounds can be open simultaneously to different servers. - PlaygroundService: per-document connection Map keyed by URI - executePlaygroundCode: per-cluster evaluator pool (one worker per cluster) - Completion/hover providers: use document URI for connection lookup - CodeLens: shows connection info, click shows notification (no connect flow) - Removed connect command, submenu; playground requires tree node to create - showWorkerStats: reports all playground workers (one per cluster)
- PlaygroundService tests: URI-based setConnection/getConnection/removeConnection, multi-document isolation, getActiveClusterIds, hasPlaygroundsForCluster - CodeLens tests: mock documents with URI, showConnectionInfo command - l10n: updated localization bundle for new/changed strings
ShellTerminalInfo now exposes clusterDisplayName, activeDatabase, isInitialized, isEvaluating, workerState, authMethod, and username. Show Worker Stats command reports all these details for shell sessions, matching the level of detail already shown for playground workers.
…levels Database: move shell from group 3 to group 5 (matching collection). Database: add inline playground button at inline@2. Cluster: add playground to context menu group 5@2.
…ive shells and playgrounds
…ionality, update the playground template
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Implements per-document Query Playground isolation by binding each playground document to its own cluster/database connection and routing execution/tooling through a URI-based connection lookup, while also pooling evaluator workers per cluster and enriching worker diagnostics.
Changes:
- Store playground connections per document URI and update UI/tooling (hover, completions, CodeLens, run commands) to resolve the connection by URI.
- Maintain a per-cluster evaluator pool and shut down evaluators whose cluster has no remaining open playground documents.
- Replace the “connect” UX with a read-only “show connection info” command and update worker stats reporting for multiple playground evaluators and richer shell session details.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/documentdb/playground/PlaygroundService.ts | Replace single global connection with per-URI connection map + close cleanup + cluster ID helpers |
| src/commands/playground/executePlaygroundCode.ts | Introduce per-cluster evaluator pool, disposal, and orphan shutdown logic; require document URI for execution |
| src/commands/playground/runAll.ts | Pass active document URI into executor and use per-URI connected check |
| src/commands/playground/runSelected.ts | Pass active document URI into executor and use per-URI connected check |
| src/commands/playground/newPlayground.ts | Require tree node and bind newly created playground document URI to its connection |
| src/documentdb/query-language/playground-completions/PlaygroundCompletionItemProvider.ts | Resolve completions using per-document connection lookup via document URI |
| src/documentdb/query-language/playground-completions/PlaygroundHoverProvider.ts | Resolve hover field lookup using per-document connection lookup via document URI |
| src/documentdb/query-language/playground-completions/CollectionNameCache.ts | Update cache warming logic to use active editor URI-based connection |
| src/documentdb/playground/PlaygroundCodeLensProvider.ts | Switch CodeLens to display-only connection info + modifier key constant |
| src/commands/playground/connectDatabase.ts | Replace connect behavior with “show connection info” notification |
| src/commands/showWorkerStats/showWorkerStats.ts | Report all playground evaluators (per cluster) + enriched shell worker stats |
| src/documentdb/shell/ShellTerminalLinkProvider.ts | Expand ShellTerminalInfo to include debug/stats metadata |
| src/documentdb/shell/DocumentDBShellPty.ts | Populate expanded ShellTerminalInfo from session manager + PTY state |
| src/documentdb/shell/ShellSessionManager.ts | Track/Expose auth method + worker state for stats |
| src/documentdb/ClustersExtension.ts | Rewire commands and evaluator shutdown lifecycle hooks |
| src/constants.ts | Add shared OS-aware modifierKey label |
| package.json | Remove connect submenu and add/show “New Query Playground” + showConnectionInfo wiring |
| l10n/bundle.l10n.json | Update/replace strings for new non-connect UX |
| docs/ai-and-plans/multi-connection-playgrounds.md | Document new architecture and lifecycle |
| docs/ai-and-plans/interactive-shell/multi-connection-behavior.md | Document runtime behavior with multiple playgrounds/shells |
Commit 5597f57 moved the trailing newline from the shell PTY into DocumentDBShellRuntime (onConsoleOutput now sends output + '\n'). The interactive shell's writeOutput() handles this correctly, but PlaygroundEvaluator was calling appendLine() which added a second newline, producing a blank line after every print() call. Switch from appendLine() to append() since the output already contains the trailing newline from the runtime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When a user closes the last playground tab for a cluster while execution is in progress, shutdownOrphanedEvaluators() would kill the running worker mid-execution, producing a confusing 'Worker exited unexpectedly' error. The choice was made to skip evaluators whose worker is in the 'executing' state. They will be cleaned up after execution completes when the next state-change event re-triggers this function. Addresses review issue #1.
When a playground execution times out, the result panel now shows the settings hint (setting key and description), mirroring the shell's behavior. The choice was made to use the same SettingsHintError metadata already produced by WorkerSessionManager and surface it in formatError. Addresses review issue #3.
The worker 'error' event handler previously only logged the error. If a worker emitted an error without immediately exiting, pending eval requests would hang until the timeout fired with a misleading 'timed out' message. The choice was made to reject all pending requests immediately with the actual error, matching the behavior of handleWorkerExit(). Addresses review issue #5.
When an uncaught exception occurs in the worker during eval, the actual error details are now sent back to the main thread as an evalError message before logging. Previously, users only saw a generic 'Worker exited unexpectedly' message and had to check the output channel for the real error. The choice was made to track the current eval requestId at module scope so the uncaught exception handler can correlate and send the error back through the normal IPC channel. Addresses review issue #6.
When a user saves a newly created playground for the first time, VS Code closes the untitled document and opens a file document with a new URI. The connection binding was lost because onDidCloseTextDocument removed the connection for the old URI. The choice was made to stash the connection briefly (keyed by fsPath) when an untitled playground closes, and migrate it to the new file URI when the corresponding file document opens. A 2-second timeout clears unclaimed stashes. Addresses review issue #9.
The context menu exposed 'New Query Playground' on cluster nodes, but the command handler only accepts DatabaseItem or CollectionItem and would fail at runtime when invoked from a cluster node. The choice was made to remove the cluster node from the menu condition rather than adding cluster-level handling, since a playground requires a specific database context. Addresses review issue #10.
The result panel was routed using vscode.window.activeTextEditor, which could point to a different document if the user switched editors during execution. The choice was made to use the documentUri parameter that is already passed to executePlaygroundCode for exactly this purpose. Addresses Copilot review comment CR-1 (PR #583).
The comment said cache was warmed 'for all connected databases' but the implementation only warms the active editor's connection. The choice was made to fix the comment to match the actual behavior. Addresses Copilot review comment CR-2 (PR #583).
The workerState and authMethod fields in ShellTerminalInfo were typed as plain strings despite having well-known constrained values. The choice was made to use inline string-literal unions for type safety. Addresses Copilot review comment CR-3 (PR #583).
Added tests covering the critical cleanup path where playground connections are removed when documents close, and the new untitled→file URI migration when a playground is saved for the first time. Addresses Copilot review comment CR-4 (PR #583).
Collaborator
Author
Review Issues AddressedFixes from the pre-merge review (
Copilot review comments addressed in individual thread replies (CR-1 through CR-4). |
Updated the review document with a complete resolution log tracking all 16 issues (12 review + 4 Copilot) with their resolution status, action taken, and commit references.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Each Query Playground document is now permanently bound to the cluster/database it was created for. Multiple playgrounds can be open simultaneously, each connected to different servers.
Changes
Per-document connections —
PlaygroundServicestores connections in aMap<URI, PlaygroundConnection>instead of a single global. Completions, hover, CodeLens, and execution all resolve the connection from the active document's URI, preventing cross-contamination between playgrounds.Per-cluster worker pool —
executePlaygroundCodemaintains aMap<clusterId, PlaygroundEvaluator>so each cluster gets its own worker thread. Workers are shut down automatically when their last playground document closes.Simplified UX — The
connectcommand and submenu are removed. Playgrounds are always created from a tree node (right-click database/collection → New Query Playground). The CodeLens on line 0 shows the connection info; clicking it shows an info notification.Enhanced worker stats —
Show Worker Statsnow reports all playground workers (one per cluster) and enriched shell worker details (auth method, database, worker state, username).Files touched
PlaygroundService.ts— per-document connection mapexecutePlaygroundCode.ts— per-cluster evaluator poolPlaygroundCompletionItemProvider.ts,PlaygroundHoverProvider.ts,CollectionNameCache.ts— URI-based connection lookupPlaygroundCodeLensProvider.ts— display-only connection lensconnectDatabase.ts→showConnectionInfo(info notification)newPlayground.ts— requires tree node, binds connection to URIrunAll.ts,runSelected.ts— pass document URIshowWorkerStats.ts— multi-evaluator + enriched shell statsShellTerminalLinkProvider.ts,ShellSessionManager.ts,DocumentDBShellPty.ts— extendedShellTerminalInfopackage.json— removed connect command, submenu; simplified menusconstants.ts,types.ts,ClustersExtension.ts— wiring updates