microsoft / microsoft/vscode-documentdb

Quality: Separate browsable cluster and stored connection context values

Open
#912 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
TypeScript
Stars
31
Forks
22
Avg merge
2d 20h
Merged PRs (30d)
21

Description

Summary

Separate the tree context that identifies a browsable cluster from the context that identifies a persisted Connections view record.

Today treeItem_documentdbcluster carries both meanings. That forces service-owned synthetic clusters to choose between inheriting incompatible storage commands or dropping the shared cluster context and duplicating each compatible command contribution.

PR #823 exposed the cost again: enabling Cluster Dashboard for DocumentDB Local requires two more explicit menu contributions even though its running node implements the shared cluster contract.

How the model grew

1. Storage-backed connections

The original Connections view cluster rows represented records in ConnectionStorageService. A single cluster context naturally implied both:

  • the node is a browsable database cluster; and
  • the node has a persisted connection record that can be renamed, moved, removed, or edited.

Commands could safely treat those properties as equivalent.

2. Browsable clusters outside connection storage

ClusterItemBase became the shared abstraction for browsable clusters across Connections, Azure, and discovery views. These nodes share database expansion, connection handling, credential access, cache identity, shell actions, and other cluster behavior, but they do not necessarily share the same persistence owner.

The context value continued to act as both a cluster capability and a storage ownership marker.

3. Synthetic managed clusters

DocumentDB Local added QuickStartClusterItem, a service-owned synthetic cluster. While running, it extends ClusterItemBase, connects through the normal cluster client path, and can support Create Database, Cluster Dashboard, Open Interactive Shell, and Refresh.

Its durable state and credentials are owned by QuickStartService, not ConnectionStorageService. It has no persisted connection record for storage-backed commands to mutate. Its stopped and transitional states are also plain lifecycle rows rather than cluster nodes.

To prevent rename, move, remove, credential editing, connection-string editing, and Data Migration commands from reaching an incompatible node, the running Quick Start item deliberately replaces the inherited treeItem_documentdbcluster context with treeItem_quickStartInstance;state_running.

Compatible cluster commands are then opted in one by one in package.json. Commit d971af0e introduced this fail-closed policy. Decision 0004 in docs/ai-and-plans/features/local-quickstart/decisions.md records the rationale and this deferred replacement.

Problem

The current policy is safe but increasingly expensive:

  • Every new shared cluster command needs a duplicate Quick Start contribution.
  • A missed opt-in creates inconsistent behavior between visually equivalent cluster rows.
  • Granting the broad cluster context exposes commands whose storage assumptions do not hold.
  • Duplicate contributions can create duplicate menu entries or ordering drift.
  • QuickStartClusterItem must remove context supplied by its base class, contrary to the desired cluster-node contract.
  • Tests need to maintain an allowlist of individual commands rather than assert stable capabilities.

The implementation difference is storage ownership, not database behavior. The context model should express that distinction directly.

Proposed direction

  1. Inventory commands gated by treeItem_documentdbcluster and classify each by the capability it requires.
  2. Keep treeItem_documentdbcluster as the marker for a browsable cluster implementing the shared cluster contract.
  3. Introduce a positive context value for persisted Connections view records, for example treeItem_storedConnection. Final naming is part of this work.
  4. Add the storage context only to cluster items backed by ConnectionStorageService.
  5. Gate rename, move, remove, update credentials, update connection string, Data Migration, and any other storage-backed commands on the storage context.
  6. Preserve the inherited cluster context on the running QuickStartClusterItem.
  7. Remove Quick Start contributions that merely duplicate generic cluster commands.
  8. Keep Quick Start lifecycle commands explicitly state-gated for stopped, starting, stopping, missing, and error rows.
  9. Audit Azure and discovery cluster nodes so command visibility follows capabilities rather than provider or tree location.

Copy Connection String nuance

Copy Connection String needs separate treatment during migration:

  • The generic cluster command can serve the running Quick Start node through getCredentials().
  • The Quick Start-specific command must remain available while stopped, when the row is not a cluster item.
  • The final menu rules must avoid showing both commands on the running row.

Acceptance criteria

  • Every browsable ClusterItemBase node retains treeItem_documentdbcluster.
  • Storage-backed commands require a positive persisted-connection context.
  • Storage-backed commands do not appear on Quick Start, Azure, discovery, or other synthetic nodes without a compatible storage record.
  • The running DocumentDB Local node receives shared cluster commands exactly once.
  • Create Database, Cluster Dashboard, Open Interactive Shell, and Refresh no longer need Quick Start-specific duplicate contributions.
  • Copy Connection String remains available in both running and stopped states without duplicate entries.
  • Non-running Quick Start lifecycle rows never receive commands that dereference a cluster model.
  • Manifest tests cover both capability inclusion and storage-command exclusion using rendered context values.
  • The tree cluster architecture guidance and Local Quick Start decision 0004 are updated when the migration lands.

Non-goals

  • Moving Quick Start state or credentials into ConnectionStorageService.
  • Changing Quick Start container lifecycle behavior.
  • Making stopped or transitional lifecycle rows browsable.
  • Reworking command implementations that already consume only the shared ClusterItemBase contract.

References

  • PR #823
  • Commit d971af0e
  • docs/ai-and-plans/features/local-quickstart/decisions.md, decision 0004
  • .github/skills/tree-cluster-architecture/SKILL.md

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading the tree-cluster architecture guidance, decision 0004 in docs/ai-and-plans/features/local-quickstart/decisions.md, and the package.json command contributions. Inventory the treeItem_documentdbcluster gates and related manifest tests, then verify that rendered contexts give shared cluster commands exactly once while storage commands stay off synthetic nodes; update the cited guidance when the migration is complete.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript, vscode
Domain
devtools, tooling
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.