Refactor base cluster model architecture for clarity

.github/copilot-instructions.md

@@ -94,32 +94,40 @@ src/commands/yourCommand/
 
 ## Cluster ID Architecture (Dual ID Pattern)
 
-`ClusterModel` has two distinct ID properties - using the wrong one causes bugs:
+> ⚠️ **CRITICAL**: Using the wrong ID causes silent bugs that only appear when users move connections between folders.
 
-| Property    | Purpose                          | Stable?                   | Example                           |
-| ----------- | -------------------------------- | ------------------------- | --------------------------------- |
-| `treeId`    | VS Code TreeView element path    | ❌ Changes on folder move | `connectionsView/folder1/abc-123` |
-| `clusterId` | Cache key (credentials, clients) | ✅ Always stable          | `abc-123` or Azure Resource ID    |
+Cluster models have **two distinct ID properties** with different purposes:
 
-### When to Use Each ID
+| Property    | Purpose                          | Stable?                   | Use For                             |
+| ----------- | -------------------------------- | ------------------------- | ----------------------------------- |
+| `treeId`    | VS Code TreeView element path    | ❌ Changes on folder move | `this.id`, child item paths         |
+| `clusterId` | Cache key (credentials, clients) | ✅ Always stable          | `CredentialCache`, `ClustersClient` |
+
+### Quick Reference
 
 ```typescript
-// ✅ Tree element identification (VS Code TreeView)
+// ✅ Tree element identification
 this.id = cluster.treeId;
-this.id = `${cluster.treeId}/${databaseInfo.name}`;
 
-// ✅ Cache operations (credentials, clients)
+// ✅ Cache operations - ALWAYS use clusterId
 CredentialCache.hasCredentials(cluster.clusterId);
 ClustersClient.getClient(cluster.clusterId);
 
-// ❌ WRONG - Don't use treeId for caching (breaks on folder move)
+// ❌ WRONG - breaks when connection moves to a folder
 CredentialCache.hasCredentials(this.id); // BUG!
-ClustersClient.getClient(cluster.treeId); // BUG!
 ```
 
-### Azure Resources View
+### Model Types
+
+- **`ConnectionClusterModel`** - Connections View (has `storageId`)
+- **`AzureClusterModel`** - Azure/Discovery Views (has Azure `id`)
+- **`BaseClusterModel`** - Shared interface (use for generic code)
+
+For Discovery View, both `treeId` and `clusterId` are sanitized (all `/` replaced with `_`). The original Azure Resource ID is stored in `AzureClusterModel.id` for Azure API calls.
+
+> 💡 **Extensibility**: If adding a non-Azure discovery source (e.g., AWS, GCP), consider creating a new model type (e.g., `AwsClusterModel`) extending `BaseClusterModel` with source-specific metadata.
 
-For Azure resources, `treeId === clusterId` (both are the Azure Resource ID). This is correct - Azure Resource IDs are already stable.
+See `src/tree/models/BaseClusterModel.ts` and `docs/analysis/08-cluster-model-simplification-plan.md` for details.
 
 ## Additional Patterns
 

src/commands/launchShell/launchShell.ts

@@ -18,6 +18,7 @@ import { RUResourceItem } from '../../tree/azure-resources-view/mongo-ru/RUCoreR
 import { ClusterItemBase } from '../../tree/documentdb/ClusterItemBase';
 import { type CollectionItem } from '../../tree/documentdb/CollectionItem';
 import { type DatabaseItem } from '../../tree/documentdb/DatabaseItem';
+import { type EmulatorConfiguration } from '../../utils/emulatorConfiguration';
 
 /**
  * Currently it only supports launching the MongoDB shell
@@ -197,12 +198,14 @@ export async function launchShell(
 
     // Determine if TLS certificate validation should be disabled
     // This only applies to emulator connections with security disabled
+    // emulatorConfiguration is only available on ConnectionClusterModel (Connections View)
     const isRegularCloudAccount = node instanceof VCoreResourceItem || node instanceof RUResourceItem;
+    const emulatorConfig: EmulatorConfiguration | undefined =
+        'emulatorConfiguration' in node.cluster
+            ? (node.cluster.emulatorConfiguration as EmulatorConfiguration)
+            : undefined;
     const isEmulatorWithSecurityDisabled =
-        !isRegularCloudAccount &&
-        node.cluster.emulatorConfiguration &&
-        node.cluster.emulatorConfiguration.isEmulator &&
-        node.cluster.emulatorConfiguration.disableEmulatorSecurity;
+        !isRegularCloudAccount && emulatorConfig?.isEmulator && emulatorConfig?.disableEmulatorSecurity;
 
     const tlsConfiguration = isEmulatorWithSecurityDisabled ? '--tlsAllowInvalidCertificates' : '';
 

src/documentdb/CredentialCache.test.ts

@@ -98,17 +98,19 @@ describe('Credential Cache Stability', () => {
             expect(connectionsModel.treeId).toContain(connectionsModel.clusterId);
         });
 
-        it('should allow treeId === clusterId for Azure resources', () => {
-            // Azure Resources View: both IDs are the Azure Resource ID
+        it('should allow treeId === clusterId for Azure resources (both sanitized)', () => {
+            // Azure views: both IDs are the sanitized Azure Resource ID (/ replaced with _)
             const azureResourceId =
                 '/subscriptions/sub1/resourceGroups/rg1/providers/Microsoft.DocumentDB/mongoClusters/mycluster';
+            const sanitizedId = azureResourceId.replace(/\//g, '_');
             const azureModel = {
-                treeId: azureResourceId,
-                clusterId: azureResourceId,
+                treeId: sanitizedId,
+                clusterId: sanitizedId,
                 name: 'mycluster',
             };
 
             expect(azureModel.treeId).toBe(azureModel.clusterId);
+            expect(azureModel.clusterId).not.toContain('/');
         });
 
         it('should change treeId but preserve clusterId when moving between folders', () => {

src/documentdb/scrapbook/ScrapbookService.ts

@@ -8,7 +8,7 @@ import * as l10n from '@vscode/l10n';
 import { EOL } from 'os';
 import * as vscode from 'vscode';
 import { ext } from '../../extensionVariables';
-import { type ClusterModel } from '../../tree/documentdb/ClusterModel';
+import { type BaseClusterModel, type TreeCluster } from '../../tree/models/BaseClusterModel';
 import { type EmulatorConfiguration } from '../../utils/emulatorConfiguration';
 import { type DatabaseItemModel } from '../ClustersClient';
 import { CredentialCache } from '../CredentialCache';
@@ -23,7 +23,7 @@ export class ScrapbookServiceImpl {
     // Connection Management
     //--------------------------------------------------------------------------------
 
-    private _cluster: ClusterModel | undefined;
+    private _cluster: TreeCluster<BaseClusterModel> | undefined;
     private _database: DatabaseItemModel | undefined;
     private readonly _mongoCodeLensProvider = new MongoCodeLensProvider();
 
@@ -37,7 +37,7 @@ export class ScrapbookServiceImpl {
     /**
      * Sets the current cluster and database, updating the CodeLens provider.
      */
-    public async setConnectedCluster(cluster: ClusterModel, database: DatabaseItemModel) {
+    public async setConnectedCluster(cluster: TreeCluster<BaseClusterModel>, database: DatabaseItemModel) {
         if (CredentialCache.getCredentials(cluster.clusterId)?.authMechanism !== AuthMethodId.NativeAuth) {
             throw Error(
                 l10n.t('Unsupported authentication mechanism. Only SCRAM-SHA-256 (username/password) is supported.'),
@@ -51,10 +51,15 @@ export class ScrapbookServiceImpl {
 
         // Update the Language Client/Server
         // The language server needs credentials to connect to the cluster..
+        // emulatorConfiguration is only available on ConnectionClusterModel (Connections View)
+        const emulatorConfig =
+            'emulatorConfiguration' in cluster
+                ? (cluster.emulatorConfiguration as EmulatorConfiguration | undefined)
+                : undefined;
         await ext.mongoLanguageClient.connect(
             CredentialCache.getConnectionStringWithPassword(this._cluster.clusterId),
             this._database.name,
-            cluster.emulatorConfiguration,
+            emulatorConfig,
         );
     }
 
@@ -83,10 +88,10 @@ export class ScrapbookServiceImpl {
     }
 
     /**
-     * Returns the current cluster ID.
+     * Returns the current cluster ID (stable identifier for caching).
      */
     public getClusterId(): string | undefined {
-        return this._cluster?.id;
+        return this._cluster?.clusterId;
     }
 
     /**

src/plugins/service-azure-mongo-ru/discovery-tree/AzureMongoRUSubscriptionItem.ts

@@ -9,10 +9,15 @@ import { callWithTelemetryAndErrorHandling, type IActionContext } from '@microso
 import { type AzureSubscription } from '@microsoft/vscode-azureresources-api';
 import * as vscode from 'vscode';
 import { CosmosDBMongoRUExperience } from '../../../DocumentDBExperiences';
+import { Views } from '../../../documentdb/Views';
 import { ext } from '../../../extensionVariables';
 import { type TreeElement } from '../../../tree/TreeElement';
 import { type TreeElementWithContextValue } from '../../../tree/TreeElementWithContextValue';
-import { type ClusterModel } from '../../../tree/documentdb/ClusterModel';
+import {
+    type AzureClusterModel,
+    sanitizeAzureResourceIdForTreeId,
+} from '../../../tree/azure-views/models/AzureClusterModel';
+import { type TreeCluster } from '../../../tree/models/BaseClusterModel';
 import { createCosmosDBManagementClient } from '../../../utils/azureClients';
 import { nonNullProp } from '../../../utils/nonNull';
 import { DISCOVERY_PROVIDER_ID } from '../config';
@@ -58,15 +63,30 @@ export class AzureMongoRUSubscriptionItem implements TreeElement, TreeElementWit
                     .map((account) => {
                         const resourceId = nonNullProp(account, 'id', 'account.id', 'AzureMongoRUSubscriptionItem.ts');
 
-                        // treeId: Replace '/' with '-' to avoid path separator issues in tree lookups
-                        // clusterId: Keep as-is (Azure Resource ID) for cache key lookups
-                        const clusterInfo: ClusterModel = {
-                            ...account,
-                            treeId: resourceId.replace(/\//g, '-'),
-                            clusterId: resourceId,
-                            resourceGroup: getResourceGroupFromId(resourceId),
+                        // Sanitize Azure Resource ID: replace '/' with '_' for treeId
+                        // This ensures treeId never contains '/' (simplifies path handling)
+                        const sanitizedId = sanitizeAzureResourceIdForTreeId(resourceId);
+
+                        // clusterId must be prefixed with provider ID for uniqueness across plugins
+                        const prefixedClusterId = `${DISCOVERY_PROVIDER_ID}_${sanitizedId}`;
+
+                        const clusterInfo: TreeCluster<AzureClusterModel> = {
+                            // Core cluster data
+                            name: account.name ?? 'Unknown',
+                            connectionString: undefined, // Loaded lazily when connecting
                             dbExperience: CosmosDBMongoRUExperience,
-                        } as ClusterModel;
+                            clusterId: prefixedClusterId, // Prefixed with provider ID for uniqueness
+                            // Azure-specific data
+                            id: resourceId, // Keep original Azure Resource ID for ARM API correlation
+                            resourceGroup: getResourceGroupFromId(resourceId),
+                            // Tree context - treeId includes parent hierarchy for findNodeById to work
+                            treeId: `${this.id}/${sanitizedId}`,
+                            viewId: Views.DiscoveryView,
+                        };
+
+                        ext.outputChannel.trace(
+                            `[DiscoveryView/MongoRU] Created cluster model: name="${clusterInfo.name}", clusterId="${clusterInfo.clusterId}", treeId="${clusterInfo.treeId}"`,
+                        );
 
                         return new MongoRUResourceItem(
                             this.journeyCorrelationId,

src/plugins/service-azure-mongo-ru/discovery-tree/documentdb/MongoRUResourceItem.ts

@@ -11,12 +11,13 @@ import { ClustersClient } from '../../../../documentdb/ClustersClient';
 import { CredentialCache } from '../../../../documentdb/CredentialCache';
 import { Views } from '../../../../documentdb/Views';
 import { ext } from '../../../../extensionVariables';
+import { type AzureClusterModel } from '../../../../tree/azure-views/models/AzureClusterModel';
 import { ClusterItemBase, type EphemeralClusterCredentials } from '../../../../tree/documentdb/ClusterItemBase';
-import { type ClusterModel } from '../../../../tree/documentdb/ClusterModel';
+import { type TreeCluster } from '../../../../tree/models/BaseClusterModel';
 import { DISCOVERY_PROVIDER_ID, RESOURCE_TYPE } from '../../config';
 import { extractCredentialsFromRUAccount } from '../../utils/ruClusterHelpers';
 
-export class MongoRUResourceItem extends ClusterItemBase {
+export class MongoRUResourceItem extends ClusterItemBase<AzureClusterModel> {
     iconPath = vscode.Uri.joinPath(
         ext.context.extensionUri,
         'resources',
@@ -35,7 +36,7 @@ export class MongoRUResourceItem extends ClusterItemBase {
          */
         journeyCorrelationId: string,
         readonly subscription: AzureSubscription,
-        cluster: ClusterModel,
+        cluster: TreeCluster<AzureClusterModel>,
     ) {
         super(cluster);
         this.journeyCorrelationId = journeyCorrelationId;

src/plugins/service-azure-mongo-vcore/discovery-tree/AzureSubscriptionItem.ts

@@ -9,10 +9,15 @@ import { callWithTelemetryAndErrorHandling, type IActionContext } from '@microso
 import { type AzureSubscription } from '@microsoft/vscode-azureresources-api';
 import * as vscode from 'vscode';
 import { DocumentDBExperience } from '../../../DocumentDBExperiences';
+import { Views } from '../../../documentdb/Views';
 import { ext } from '../../../extensionVariables';
 import { type TreeElement } from '../../../tree/TreeElement';
 import { type TreeElementWithContextValue } from '../../../tree/TreeElementWithContextValue';
-import { type ClusterModel } from '../../../tree/documentdb/ClusterModel';
+import {
+    type AzureClusterModel,
+    sanitizeAzureResourceIdForTreeId,
+} from '../../../tree/azure-views/models/AzureClusterModel';
+import { type TreeCluster } from '../../../tree/models/BaseClusterModel';
 import { createResourceManagementClient } from '../../../utils/azureClients';
 import { nonNullProp } from '../../../utils/nonNull';
 import { DISCOVERY_PROVIDER_ID } from '../config';
@@ -60,15 +65,30 @@ export class AzureSubscriptionItem implements TreeElement, TreeElementWithContex
                     .map((account) => {
                         const resourceId = nonNullProp(account, 'id', 'account.id', 'AzureSubscriptionItem.ts');
 
-                        // treeId: Replace '/' with '-' to avoid path separator issues in tree lookups
-                        // clusterId: Keep as-is (Azure Resource ID) for cache key lookups
-                        const clusterInfo: ClusterModel = {
-                            ...account,
-                            treeId: resourceId.replace(/\//g, '-'),
-                            clusterId: resourceId,
-                            resourceGroup: getResourceGroupFromId(resourceId),
+                        // Sanitize Azure Resource ID: replace '/' with '_' for treeId
+                        // This ensures treeId never contains '/' (simplifies path handling)
+                        const sanitizedId = sanitizeAzureResourceIdForTreeId(resourceId);
+
+                        // clusterId must be prefixed with provider ID for uniqueness across plugins
+                        const prefixedClusterId = `${DISCOVERY_PROVIDER_ID}_${sanitizedId}`;
+
+                        const clusterInfo: TreeCluster<AzureClusterModel> = {
+                            // Core cluster data
+                            name: account.name ?? 'Unknown',
+                            connectionString: undefined, // Loaded lazily when connecting
                             dbExperience: DocumentDBExperience,
-                        } as ClusterModel;
+                            clusterId: prefixedClusterId, // Prefixed with provider ID for uniqueness
+                            // Azure-specific data
+                            id: resourceId, // Keep original Azure Resource ID for ARM API correlation
+                            resourceGroup: getResourceGroupFromId(resourceId),
+                            // Tree context - treeId includes parent hierarchy for findNodeById to work
+                            treeId: `${this.id}/${sanitizedId}`,
+                            viewId: Views.DiscoveryView,
+                        };
+
+                        ext.outputChannel.trace(
+                            `[DiscoveryView/vCore] Created cluster model: name="${clusterInfo.name}", clusterId="${clusterInfo.clusterId}", treeId="${clusterInfo.treeId}"`,
+                        );
 
                         return new DocumentDBResourceItem(
                             this.journeyCorrelationId,

src/plugins/service-azure-mongo-vcore/discovery-tree/documentdb/DocumentDBResourceItem.ts

@@ -22,14 +22,15 @@ import { ChooseAuthMethodStep } from '../../../../documentdb/wizards/authenticat
 import { ProvidePasswordStep } from '../../../../documentdb/wizards/authenticate/ProvidePasswordStep';
 import { ProvideUserNameStep } from '../../../../documentdb/wizards/authenticate/ProvideUsernameStep';
 import { ext } from '../../../../extensionVariables';
+import { type AzureClusterModel } from '../../../../tree/azure-views/models/AzureClusterModel';
 import { ClusterItemBase, type EphemeralClusterCredentials } from '../../../../tree/documentdb/ClusterItemBase';
-import { type ClusterModel } from '../../../../tree/documentdb/ClusterModel';
+import { type TreeCluster } from '../../../../tree/models/BaseClusterModel';
 import { getThemeAgnosticIconPath } from '../../../../utils/icons';
 import { nonNullValue } from '../../../../utils/nonNull';
 import { DISCOVERY_PROVIDER_ID, RESOURCE_TYPE } from '../../config';
 import { extractCredentialsFromCluster, getClusterInformationFromAzure } from '../../utils/clusterHelpers';
 
-export class DocumentDBResourceItem extends ClusterItemBase {
+export class DocumentDBResourceItem extends ClusterItemBase<AzureClusterModel> {
     iconPath = getThemeAgnosticIconPath('AzureDocumentDb.svg');
 
     constructor(
@@ -39,7 +40,7 @@ export class DocumentDBResourceItem extends ClusterItemBase {
          */
         journeyCorrelationId: string,
         readonly subscription: AzureSubscription,
-        cluster: ClusterModel,
+        cluster: TreeCluster<AzureClusterModel>,
     ) {
         super(cluster);
         this.journeyCorrelationId = journeyCorrelationId;