chore: code review comment

Author: saltenasl

src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts

@@ -63,14 +63,13 @@ export class DeepnoteEnvironmentManager implements IExtensionSyncActivationServi
                 const actualVenvParent = path.dirname(config.venvPath.fsPath);
                 const isInCorrectStorage = actualVenvParent === expectedVenvParent;
 
-                const uuidPattern = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
-                const isHashBasedName = !uuidPattern.test(venvDirName);
-
-                const needsPathMigration = isHashBasedName || !isInCorrectStorage;
+                // Check if directory name matches the environment ID and is in correct storage
+                const isExpectedPath = venvDirName === config.id && isInCorrectStorage;
+                const needsPathMigration = !isExpectedPath;
 
                 if (needsPathMigration) {
                     logger.info(
-                        `Migrating environment "${config.name}" from old venv path ${config.venvPath.fsPath} to UUID-based path`
+                        `Migrating environment "${config.name}" from ${config.venvPath.fsPath} to ID-based path`
                     );
 
                     config.venvPath = Uri.joinPath(this.context.globalStorageUri, 'deepnote-venvs', config.id);

src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.ts

@@ -335,7 +335,7 @@ suite('DeepnoteEnvironmentManager', () => {
             verify(mockStorage.saveEnvironments(anything())).once();
         });
 
-        test('should not migrate environments with correct UUID paths in correct storage', async () => {
+        test('should not migrate environments with correct ID-based paths in correct storage', async () => {
             const testDate = new Date();
             const correctConfig = {
                 id: '12345678-1234-1234-1234-123456789abc',
@@ -372,6 +372,38 @@ suite('DeepnoteEnvironmentManager', () => {
             verify(mockStorage.saveEnvironments(anything())).never();
         });
 
+        test('should not migrate environments with non-UUID IDs when path already matches', async () => {
+            const testDate = new Date();
+            const customIdConfig = {
+                id: 'my-custom-env-id',
+                name: 'Custom ID Environment',
+                pythonInterpreter: testInterpreter,
+                venvPath: Uri.file('/global/storage/deepnote-venvs/my-custom-env-id'),
+                createdAt: testDate,
+                lastUsedAt: testDate,
+                toolkitVersion: '1.0.0'
+            };
+
+            when(mockStorage.loadEnvironments()).thenResolve([customIdConfig]);
+            when(mockStorage.saveEnvironments(anything())).thenResolve();
+            when(mockContext.globalStorageUri).thenReturn(Uri.file('/global/storage'));
+
+            manager.activate();
+            await manager.waitForInitialization();
+
+            const configs = manager.listEnvironments();
+            assert.strictEqual(configs.length, 1);
+
+            // Path should remain unchanged
+            assert.strictEqual(configs[0].venvPath.fsPath, '/global/storage/deepnote-venvs/my-custom-env-id');
+
+            // Toolkit version should NOT be cleared
+            assert.strictEqual(configs[0].toolkitVersion, '1.0.0');
+
+            // Should NOT have saved (no migration needed)
+            verify(mockStorage.saveEnvironments(anything())).never();
+        });
+
         test('should migrate multiple environments at once', async () => {
             const configs = [
                 {