fix: migrate venv paths for cross-editor compatibility and legacy naming

[saltenasl]

This occurs because:

1. Cross-editor contamination: Notebooks opened in VS Code cache controller metadata with VS Code storage paths, which fail when opened in Cursor
2. Legacy naming: Old versions used hash-based venv names (venv_*) instead of UUIDs
3. Stale cached controllers: Notebook controllers persist in memory with outdated interpreter paths

Solution

1. Automatic Environment Migration (deepnoteEnvironmentManager.node.ts)

• Detects environments with hash-based venv paths and migrates to UUID-based naming
• Detects environments from different editor storage (VS Code vs Cursor) and updates paths
• Clears toolkit version to force reinstallation in new location
• Logs all migration activities

2. Controller Validation (deepnoteKernelAutoSelector.node.ts)

• Before reusing cached controllers, validates interpreter paths match expected venv location
• Automatically disposes and recreates controllers with wrong paths

3. Path Validation (deepnoteToolkitInstaller.node.ts)

• Validates venv paths are in current globalStorageUri before use
• Throws clear errors if path mismatches are detected

4. Enhanced Logging

• Logs all venv paths, base interpreters, and migration activities

Testing

Added comprehensive unit tests (all passing ✅):

• Hash-based to UUID migration
• Cross-editor storage migration (VS Code → Cursor)
• No-op for correct paths
• Bulk migration of multiple environments

Impact

✅ Fixes venv path errors when switching between editors
✅ Fixes compatibility issues with old extension versions
✅ Automatic migration - no user action required
✅ Clear error messages for better debugging

Summary by CodeRabbit

• Bug Fixes

  • Enhanced virtual environment path validation to detect and prevent cross-storage mismatches
  • Added automatic migration of legacy venv paths to the standardized storage layout and clearing stale toolkit info
  • Improved kernel controller verification to ensure controllers match the expected venv, disposing mismatched controllers
  • Added diagnostic logging around venv and interpreter paths for better traceability

• Tests

  • Added comprehensive test suite for environment migration scenarios

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds runtime validation in the toolkit installer to ensure venv paths are under the extension's globalStorageUri and throws/logs on mismatch. Environment manager now detects outdated venv paths (hash-based or stored under the wrong global storage), migrates them to an ID-based path under globalStorageUri, clears toolkitVersion, and persists changes when migrations occur. Kernel auto-selector validates a controller's interpreter path against the expected venv and disposes mismatched controllers to force recreation. Unit tests cover migration scenarios and save behavior.

Sequence Diagram(s)

sequenceDiagram
    participant Init as Initialization
    participant EnvMgr as DeepnoteEnvironmentManager
    participant Installer as DeepnoteToolkitInstaller
    participant Selector as DeepnoteKernelAutoSelector

    rect rgb(230,240,255)
    Note over Init,EnvMgr: Environment migration
    Init->>EnvMgr: initEnvironments()
    EnvMgr->>EnvMgr: For each env: check venvPath basename and storage prefix
    alt Outdated (hash-based or wrong storage)
        EnvMgr->>EnvMgr: Compute ID-based path under globalStorageUri
        EnvMgr->>EnvMgr: Clear toolkitVersion
        EnvMgr->>EnvMgr: Mark for save
    else Up-to-date
        EnvMgr->>EnvMgr: No-op
    end
    EnvMgr->>EnvMgr: Persist if any migrations
    end

    rect rgb(220,255,230)
    Note over Installer: Venv validation before install
    Init->>Installer: ensureVenvAndToolkit(env)
    Installer->>Installer: Generate venv key and candidate path
    Installer->>Installer: Log base interpreter path
    Installer->>Installer: Validate candidate under globalStorageUri
    alt Invalid location
        Installer->>Installer: Log error and throw (cross-editor mismatch)
    else Valid location
        Installer->>Installer: Continue pending-install checks and install
    end
    end

    rect rgb(255,245,215)
    Note over Selector: Controller interpreter guard
    Init->>Selector: ensureKernelSelectedWithConfiguration(env)
    Selector->>Selector: Find existing controller for env
    alt Controller found
        Selector->>Selector: Compare controller.interpreterPath vs expected venv interpreter
        alt Mismatch
            Selector->>Selector: Dispose & remove controller -> force recreate
        else Match
            Selector->>Selector: Reuse controller
        end
    else No controller
        Selector->>Selector: Create new controller
    end
    end

Loading

Possibly related PRs

• feat: Deepnote kernel management view #52: Modifies the same Deepnote environment and venv management areas (installer, environment manager, kernel selector).
• Add Deepnote Environments kernel management #152: Earlier changes to environment/venv management that this PR refines (migration and validation logic).
• fix: add pip upgrading to deepnote toolkit installation #20: Related changes in the toolkit installer affecting venv creation/install flow.

Suggested reviewers

• Artmann
• tkislan
• m1so

Pre-merge checks

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	Title directly summarizes the main change: migrating venv paths for cross-editor compatibility and legacy naming, matching the PR objectives.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 627fc5d and d191542.

📒 Files selected for processing (4)

• src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (1 hunks)
• src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts (2 hunks)
• src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.ts (1 hunks)
• src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (2 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

src/kernels/**/*.ts

📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)

src/kernels/**/*.ts: Use event-driven updates (EventEmitter) for state changes
Monitor and dispose pending promises to prevent leaks during teardown
Use WeakRef/weak references for notebook/object backreferences to avoid memory leaks
Respect CancellationToken in all async operations

Files:

• src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts
• src/kernels/deepnote/deepnoteToolkitInstaller.node.ts
• src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.ts

**/*.{test,spec}.ts

📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)

In unit tests, when a mock is returned from a promise, ensure the mocked instance has an undefined then property to avoid hanging tests

Files:

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

🧬 Code graph analysis (3)

src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts (1)

src/platform/logging/index.ts (1)

• logger (35-48)

src/kernels/deepnote/deepnoteToolkitInstaller.node.ts (1)

src/platform/logging/index.ts (1)

• logger (35-48)

src/notebooks/deepnote/deepnoteKernelAutoSelector.node.ts (1)

src/platform/logging/index.ts (1)

• logger (35-48)

[codecov]

Codecov Report

❌ Patch coverage is 86.31579% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 73%. Comparing base (5bdf938) to head (2f5f736).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...ebooks/deepnote/deepnoteKernelAutoSelector.node.ts	0%	13 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff          @@
##            main    #188   +/-   ##
=====================================
  Coverage     73%     73%           
=====================================
  Files        575     575           
  Lines      46923   47015   +92     
  Branches    5521    5527    +6     
=====================================
+ Hits       34490   34572   +82     
- Misses     10611   10621   +10     
  Partials    1822    1822           

Files with missing lines	Coverage Δ
...te/environments/deepnoteEnvironmentManager.node.ts	90% <100%> (+2%)	⬆️
...vironments/deepnoteEnvironmentManager.unit.test.ts	100% <100%> (ø)
...ebooks/deepnote/deepnoteKernelAutoSelector.node.ts	27% <0%> (-1%)	⬇️

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between d191542 and e09011e.

📒 Files selected for processing (2)

• src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts (2 hunks)
• src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

src/kernels/**/*.ts

📄 CodeRabbit inference engine (.github/instructions/kernel.instructions.md)

src/kernels/**/*.ts: Use event-driven updates (EventEmitter) for state changes
Monitor and dispose pending promises to prevent leaks during teardown
Use WeakRef/weak references for notebook/object backreferences to avoid memory leaks
Respect CancellationToken in all async operations

Files:

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

**/*.{test,spec}.ts

📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)

In unit tests, when a mock is returned from a promise, ensure the mocked instance has an undefined then property to avoid hanging tests

Files:

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

🧬 Code graph analysis (1)

src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts (1)

src/platform/logging/index.ts (1)

• logger (35-48)

🔇 Additional comments (4)

src/kernels/deepnote/environments/deepnoteEnvironmentManager.node.ts (1)

2-2: LGTM!

Path module needed for basename/dirname operations in migration logic.

src/kernels/deepnote/environments/deepnoteEnvironmentManager.unit.test.ts (3)

304-336: Cross-editor storage migration test looks good.

Validates VS Code→Cursor migration with a non-UUID ID, ensuring the logic works for arbitrary IDs. Regex match appropriately handles platform-specific paths.

---

338-405: No-op tests ensure efficiency.

Both tests verify that environments with correct paths don't trigger unnecessary migrations or saves. Test 4 (custom ID) is particularly valuable—it confirms the migration logic works for any ID format, not just UUIDs.

---

407-452: Batch migration test validates efficiency.

Confirms that multiple environments are migrated in a single pass with one save operation, not multiple saves.

[Artmann]

Why are we cleaning the toolkit version?