fix(deepnote-server): ensure consecutive port allocation skips bound ports

[saltenasl]

• replace get-port use with real availability checks via tcp-port-used so occupied ports are never selected
• ensure findConsecutiveAvailablePorts and allocatePorts retry until they find true consecutive pairs
• add regression tests that bind to ports and verify both availability detection and consecutive allocation logic

Can be tested by running python3 -m http.server 8889 in terminal and then starting the extension - server should start successfully on port 8890 (and lsp on 8891). This previously would fail to start the server.

Summary by CodeRabbit

• Improvements

  • Port allocation now finds and atomically reserves consecutive port pairs, verifies runtime availability, serializes concurrent allocations, and updates logs to reflect consecutive allocations.
  • Startup now skips external/non‑managed processes without lock files and logs skip reasons.

• Tests

  • Added extensive tests for consecutive-port allocation, real availability checks, blocking/concurrency scenarios, edge cases, and regression protection.

• Chores

  • Updated spellcheck dictionary to include "loopbacks"

[coderabbitai]

📝 Walkthrough

Walkthrough

The port allocation in DeepnoteServerStarter now reserves consecutive port pairs (jupyterPort and lspPort) using findConsecutiveAvailablePorts, which probes runtime availability via isPortAvailable and findAvailablePort before reserving. Allocation is serialized with a chainable lock to avoid races. Orphaned-process detection was simplified: processes without a lock file are treated as external and skipped. Tests were added to validate real TCP availability, consecutive-pair guarantees, non-overlap, concurrency, and edge cases.

Sequence Diagram

sequenceDiagram
    participant Starter as DeepnoteServerStarter
    participant AllocLock as AllocationLock
    participant Finder as findConsecutiveAvailablePorts
    participant Probe as isPortAvailable

    Starter->>AllocLock: acquire()
    AllocLock-->>Starter: lock acquired
    Starter->>Finder: allocatePorts(basePort)
    Finder->>Finder: attempt candidate = basePort..maxAttempts
    alt probing candidate
        Finder->>Probe: probe candidate (IPv4/IPv6)
        Probe-->>Finder: candidate available?
        alt candidate available
            Finder->>Probe: probe candidate+1 (LSP)
            Probe-->>Finder: successor available?
            alt both available
                Finder-->>Starter: return { jupyterPort: candidate, lspPort: candidate+1 }
            else
                Finder->>Finder: increment candidate
            end
        else
            Finder->>Finder: increment candidate
        end
    end
    Starter->>AllocLock: reserve ports atomically
    AllocLock-->>Starter: release()

Loading

Possibly related PRs

• deepnote/vscode-deepnote PR 52 — modifies the same Deepnote server startup port-allocation and orphaned-process handling logic.

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	The title accurately describes the main change: fixing port allocation to ensure consecutive ports and skip bound ones.

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

---

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

[codecov]

Codecov Report

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

Files with missing lines	Patch %	Lines
...ernels/deepnote/deepnoteServerStarter.unit.test.ts	98%	0 Missing and 3 partials ⚠️
src/kernels/deepnote/deepnoteServerStarter.node.ts	93%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##            main    #189    +/-   ##
======================================
- Coverage     73%     73%    -1%     
======================================
  Files        575     577     +2     
  Lines      46923   47534   +611     
  Branches    5521    5590    +69     
======================================
+ Hits       34490   34820   +330     
- Misses     10611   10877   +266     
- Partials    1822    1837    +15     

Files with missing lines	Coverage Δ
src/kernels/deepnote/deepnoteServerStarter.node.ts	24% <93%> (ø)
...ernels/deepnote/deepnoteServerStarter.unit.test.ts	98% <98%> (ø)

... and 1 file with indirect coverage changes

🚀 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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

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

502-510: Fix the port allocation lock race

The current pattern awaits an already-resolved portAllocationLock, then installs the new promise. When two callers enter together, both resume after the await before either has assigned the new promise, so the second caller also enters the critical section and can reuse the same port pair. That undermines the serialization and lets server startups collide.

Switch to the standard “remember previous lock, chain a new one, then await the previous” pattern so each caller really waits for its predecessor.

Apply this diff:

-        await this.portAllocationLock;
-
-        // Create new allocation promise and update the lock
-        let releaseLock: () => void;
-        this.portAllocationLock = new Promise((resolve) => {
-            releaseLock = resolve;
-        });
+        const previousLock = this.portAllocationLock;
+        let releaseLock: () => void;
+        const currentLock = new Promise<void>((resolve) => {
+            releaseLock = resolve;
+        });
+        this.portAllocationLock = previousLock.then(() => currentLock);
+        await previousLock;

📜 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 5bdf938 and d11e00e.

📒 Files selected for processing (2)

• src/kernels/deepnote/deepnoteServerStarter.node.ts (6 hunks)
• src/kernels/deepnote/deepnoteServerStarter.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/deepnoteServerStarter.unit.test.ts
• src/kernels/deepnote/deepnoteServerStarter.node.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/deepnoteServerStarter.unit.test.ts

🧬 Code graph analysis (2)

src/kernels/deepnote/deepnoteServerStarter.unit.test.ts (1)

src/kernels/deepnote/deepnoteServerStarter.node.ts (3)

• isPortAvailable (614-628)
• findAvailablePort (634-667)
• allocatePorts (501-553)

src/kernels/deepnote/deepnoteServerStarter.node.ts (3)

src/kernels/deepnote/types.ts (1)

• DEEPNOTE_DEFAULT_PORT (321-321)

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

• logger (35-48)

src/platform/errors/deepnoteKernelErrors.ts (1)

• DeepnoteServerStartupError (174-247)

[coderabbitai]

Actionable comments posted: 2

📜 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 5bdf938 and 868d109.

📒 Files selected for processing (2)

• src/kernels/deepnote/deepnoteServerStarter.node.ts (6 hunks)
• src/kernels/deepnote/deepnoteServerStarter.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/deepnoteServerStarter.unit.test.ts
• src/kernels/deepnote/deepnoteServerStarter.node.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/deepnoteServerStarter.unit.test.ts

🧬 Code graph analysis (2)

src/kernels/deepnote/deepnoteServerStarter.unit.test.ts (2)

src/kernels/deepnote/deepnoteServerStarter.node.ts (3)

• isPortAvailable (614-628)
• findAvailablePort (634-667)
• allocatePorts (501-553)

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

• logger (35-48)

src/kernels/deepnote/deepnoteServerStarter.node.ts (3)

src/kernels/deepnote/types.ts (1)

• DEEPNOTE_DEFAULT_PORT (321-321)

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

• logger (35-48)

src/platform/errors/deepnoteKernelErrors.ts (1)

• DeepnoteServerStartupError (174-247)

🪛 GitHub Actions: CI

src/kernels/deepnote/deepnoteServerStarter.unit.test.ts

[error] 67-67: cSpell: Unknown word 'loopbacks' in test string (Unknown word).

---

[error] 77-77: cSpell: Unknown word 'loopbacks' in test string (Unknown word).

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

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

616-629: Handle IPv6-unavailable errors without blocking ports.

On IPv4-only hosts tcpPortUsed.check(port, '::1') throws (e.g. EADDRNOTAVAIL/EAFNOSUPPORT/ENETUNREACH). We catch that, warn, and return false, so every port looks busy and allocation always fails. Treat those errno codes as “IPv6 loopback missing” and rely on the IPv4 result instead.

Apply this diff:

-        try {
-            const inUse = await tcpPortUsed.check(port, '127.0.0.1');
-            if (inUse) {
-                return false;
-            }
-
-            // Also check IPv6 loopback to be safe
-            const inUseIpv6 = await tcpPortUsed.check(port, '::1');
-            return !inUseIpv6;
-        } catch (error) {
-            logger.warn(`Failed to check port availability for ${port}:`, error);
-            return false;
-        }
+        try {
+            const inUseIpv4 = await tcpPortUsed.check(port, '127.0.0.1');
+            if (inUseIpv4) {
+                return false;
+            }
+        } catch (error) {
+            logger.warn(`Failed to check IPv4 availability for ${port}:`, error);
+            return false;
+        }
+
+        try {
+            const inUseIpv6 = await tcpPortUsed.check(port, '::1');
+            return !inUseIpv6;
+        } catch (error) {
+            const code = (error as NodeJS.ErrnoException).code;
+            if (code === 'EADDRNOTAVAIL' || code === 'EAFNOSUPPORT' || code === 'ENETUNREACH') {
+                logger.debug(`IPv6 loopback unavailable for port ${port} (${code}), using IPv4 result only`);
+                return true;
+            }
+            logger.warn(`Failed to check IPv6 availability for ${port}:`, error);
+            return false;
+        }

📜 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 f9ebd06 and f6c0c50.

📒 Files selected for processing (2)

• src/kernels/deepnote/deepnoteServerStarter.node.ts (7 hunks)
• src/kernels/deepnote/deepnoteServerStarter.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/deepnoteServerStarter.node.ts
• src/kernels/deepnote/deepnoteServerStarter.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/deepnoteServerStarter.unit.test.ts