Refactor base cluster model architecture for clarity

[tnaum-ms]

Refactor base cluster model architecture for clarity

Summary

This PR refactors the cluster model type hierarchy to cleanly separate intrinsic cluster data from tree positioning concerns, and to decouple the Connections View from Azure-specific types. It also fixes the "Could not find the specified collection in the tree" error when operating on collections from Discovery View webviews.

Problem

1. Type coupling: ClusterModel inherited Azure SDK Resource metadata, forcing all views (including Connections View) to carry Azure-specific fields.

2. ID confusion: The single id property served two incompatible purposes:

   • VS Code tree element path (hierarchical, changes when moved between folders)
   • Cache lookup key for credentials/clients (must be stable)

3. Export/import failures in Discovery View: Operations on collections opened from Discovery View clusters failed because findCollectionByClusterId couldn't translate the clusterId back to a treeId to locate the collection in the tree.

Solution

1. New Type Hierarchy

Introduce explicit separation of concerns with a new type hierarchy:

BaseClusterModel (minimal, intrinsic cluster data)
├── ConnectionClusterModel (Connections View – no Azure deps)
└── AzureClusterModel (Azure/Discovery Views – full Azure metadata)

Tree positioning is handled by ClusterTreeContext, and tree items use TreeCluster<T> to combine data + positioning.

2. Dual ID Architecture

Property	Purpose	Stable?	Example
treeId	VS Code tree element ID	❌ Changes on folder move	discoveryView/providerId/subId/clusterId
clusterId	Cache key (credentials, clients)	✅ Always stable	azure-mongo-vcore-discovery_sanitizedId

3. Discovery View Plugin Contract

Plugins must set clusterId with a {providerId}_ prefix:

// Example: vCore plugin
clusterId: `${DISCOVERY_PROVIDER_ID}_${sanitizedAzureResourceId}`
// Result: "azure-mongo-vcore-discovery__subscriptions_xxx_mongoClusters_yyy"

This enables findCollectionByClusterId to:

1. Extract the original ID (everything after the first _)
2. Find the cluster in cache via suffix matching
3. Search from the cluster using the new findChildById method

4. New findChildById Method

Added to BaseExtendedTreeDataProvider to search from a specific parent without ancestor fallback. This prevents a bug where deleted collections would trigger expansion of ALL sibling clusters in the subscription.

Key Changes

New Models:

• src/tree/models/BaseClusterModel.ts - Core cluster interface with treeId, clusterId
• src/tree/connections-view/models/ConnectionClusterModel.ts - Connections View model
• src/tree/azure-views/models/AzureClusterModel.ts - Azure Views model + sanitizeAzureResourceIdForTreeId() helper

Tree Infrastructure:

• src/tree/BaseExtendedTreeDataProvider.ts - Added findChildById() method
• src/tree/TreeParentCache.ts - Added findNodeBySuffix() method
• src/tree/discovery-view/clusterItemTypeGuard.ts - Type guard for cluster items

Discovery View:

• src/tree/discovery-view/DiscoveryBranchDataProvider.ts - Added validateClusterIdPrefix(), updated findCollectionByClusterId
• All discovery plugins updated to set prefixed clusterId

Connections View:

• src/tree/connections-view/ConnectionsBranchDataProvider.ts - Uses ConnectionClusterModel
• src/tree/connections-view/FolderItem.ts - Sets treeId and clusterId correctly

Cache Operations:

• All commands updated to use cluster.clusterId for cache lookups
• CredentialCache and ClustersClient documentation updated

ID Strategy Summary

View	clusterId (cache key)	treeId (tree element ID)
Connections View	storageId (UUID)	connectionsView/[folder/]storageId
Discovery View	{providerId}_{sanitizedId}	discoveryView/{providerId}/.../sanitizedId
Azure Resources View	{sanitizedId}	{sanitizedId} (same as clusterId)

Key constraint: Original (unsanitized) Azure Resource IDs contain / which breaks tree path parsing. We sanitize by replacing / with _.

findCollectionByClusterId Implementation

Connections View: Uses buildFullTreePath(clusterId) to reconstruct full tree path from storage.

Discovery View (fixed in this PR):

// 1. Extract original ID from prefixed clusterId
const originalClusterId = clusterId.substring(clusterId.indexOf('_') + 1);

// 2. Find cluster in cache by suffix (cache-only, no backend calls)
const clusterNode = this.findNodeBySuffix(`/${originalClusterId}`);

// 3. Search from cluster using findChildById (prevents ancestor fallback)
return this.findChildById(clusterNode, `${clusterNode.id}/${dbName}/${collName}`);

Tests

• npm run build ✅
• npm run lint ✅
• All 402 Jest tests pass ✅

Breaking Changes

• Internal only: No external API changes
• ClusterModel is now a deprecated union type pointing to the new specific types

Notes

• This continues the direction established by PR fix: maintain connections after a connection move. clusterId and treeId are now separate #472 (treeId/clusterId separation)
• The findChildById method is preferred over findNodeById when you have the parent, as it avoids ancestor fallback that can cause unintended tree expansion
• Discovery View plugins are now validated to ensure clusterId starts with providerId

[Copilot]

Pull request overview

This PR refactors the cluster model type hierarchy to cleanly separate intrinsic cluster data from tree positioning concerns, removing Azure SDK dependencies from the Connections View.

Changes:

• Introduces a new type hierarchy: BaseClusterModel (core data) → ConnectionClusterModel (Connections View) / AzureClusterModel (Azure/Discovery Views)
• Separates tree positioning into ClusterTreeContext (with treeId and viewId) and combines them via TreeCluster<T> type
• Migrates all views to use appropriate model types: Connections View uses ConnectionClusterModel, Azure/Discovery Views use AzureClusterModel, generic tree items use BaseClusterModel
• Makes ClusterItemBase generic and safely accesses Azure-specific properties via runtime checks
• Deprecates the old ClusterModel type as a union with clear migration guidance

Reviewed changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/tree/models/BaseClusterModel.ts	Defines core cluster data interface without tree positioning concerns
src/tree/models/BaseClusterModel.test.ts	Comprehensive tests for base model and tree context separation
src/tree/connections-view/models/ConnectionClusterModel.ts	Clean Connections View model with storageId and emulatorConfiguration
src/tree/connections-view/models/ConnectionClusterModel.test.ts	Tests for ConnectionClusterModel including folder hierarchy scenarios
src/tree/azure-views/models/AzureClusterModel.ts	Azure-specific model with ARM metadata and sanitization helper
src/tree/azure-views/models/AzureClusterModel.test.ts	Tests for AzureClusterModel including Discovery vs Azure Resources View patterns
src/tree/documentdb/ClusterModel.ts	Deprecated union type with clear migration guide for backward compatibility
src/tree/documentdb/ClusterItemBase.ts	Made generic, safely casts to Azure properties when needed
src/tree/connections-view/*	All Connections View files migrated to ConnectionClusterModel
src/tree/azure-resources-view/*	Azure Resources View files migrated to AzureClusterModel with proper treeId handling
src/plugins//discovery-tree/	Discovery View files migrated to AzureClusterModel with treeId sanitization
src/tree/documentdb/{DatabaseItem,CollectionItem,etc}.ts	Generic tree items updated to use BaseClusterModel
src/documentdb/scrapbook/ScrapbookService.ts	Uses BaseClusterModel with safe type narrowing for emulatorConfiguration
src/commands/launchShell/launchShell.ts	Safe type narrowing for emulatorConfiguration access
.github/copilot-instructions.md	Updated with new model types and ID strategy clarification

[Copilot]

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated 18 comments.

[Copilot]

Pull request overview

Copilot reviewed 43 out of 43 changed files in this pull request and generated 4 comments.

[tnaum-ms]

@copilot open a new pull request to apply changes based on the comments in this thread

[Copilot]

@tnaum-ms I've opened a new pull request, #476, to work on those changes. Once the pull request is ready, I'll request review from you.

[tnaum-ms]

@copilot open a new pull request to apply changes based on the comments in this thread
some comments might have been marked as resolved, ignore this status and revisit every one anyway. They might have been marked as resolved by mistake. Same applies to the comment 'stale'.

[Copilot]

@tnaum-ms I've opened a new pull request, #477, to work on those changes. Once the pull request is ready, I'll request review from you.