fix: Package the SQL language server.

[Artmann]

Includes the SQL LSP in the built package.

Summary by CodeRabbit

• Chores
  • Improved packaging and dependency handling for the SQL language server, ensuring more reliable module loading in packaged desktop builds and when running/debugging the extension.
  • Ensures the language server environment is properly resolved at runtime, reducing startup failures and improving stability.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

A new build step bundles the SQL language server into a single CommonJS file and copies its runtime dependencies into a dedicated sql-lsp-modules workspace (including generating a minimal package.json and running an npm install). The buildAll sequence now calls this bundler. At runtime the extension computes the sql-lsp-modules/node_modules path and injects it into NODE_PATH for both run and debug IPC server processes, with a documented fallback to dist/sqlLanguageServer.cjs.

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ 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 clearly summarizes the main change: packaging the SQL language server into the distribution. It aligns with both the code changes (build bundling) and PR objectives.

---

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0%. Comparing base (2590146) to head (1380b01).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@     Coverage Diff     @@
##   main   #240   +/-   ##
===========================
===========================

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

📜 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 2590146 and 8b28cf0.

📒 Files selected for processing (2)

• build/esbuild/build.ts (2 hunks)
• src/kernels/deepnote/deepnoteLspClientManager.node.ts (4 hunks)

🧰 Additional context used

📓 Path-based instructions (5)

**/*.node.ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Platform Implementation - Desktop: Use .node.ts files for full file system access and Python environments

Files:

• src/kernels/deepnote/deepnoteLspClientManager.node.ts

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Use l10n.t() for all user-facing strings in TypeScript files
Use typed error classes from src/platform/errors/ instead of generic errors
Use ILogger service instead of console.log for logging
Preserve error details in error messages while scrubbing personally identifiable information (PII)
Prefer async/await over promise chains
Handle cancellation with CancellationToken

Order method, fields and properties first by accessibility (public/private/protected) and then by alphabetical order

Files:

• src/kernels/deepnote/deepnoteLspClientManager.node.ts
• build/esbuild/build.ts

src/kernels/**/*.ts

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

src/kernels/**/*.ts: Respect CancellationToken in all async operations across kernel execution, session communication, and finder operations
Implement proper error handling in files using custom error types from errors/ directory (KernelDeadError, KernelConnectionTimeoutError, KernelInterruptTimeoutError, JupyterInvalidKernelError, KernelDependencyError)

Files:

• src/kernels/deepnote/deepnoteLspClientManager.node.ts

**/*.ts

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

**/*.ts: ALWAYS check the 'Core - Build' task output for TypeScript compilation errors before running any script or declaring work complete
ALWAYS run npm run format-fix before committing changes to ensure proper code formatting
FIX all TypeScript compilation errors before moving forward with development
Use npm run format-fix to auto-fix TypeScript formatting issues before committing
Use npm run lint to check for linter issues in TypeScript files and attempt to fix before committing

Files:

• src/kernels/deepnote/deepnoteLspClientManager.node.ts
• build/esbuild/build.ts

src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{ts,tsx}: Don't add the Microsoft copyright header to new files
Use Uri.joinPath() for constructing file paths instead of string concatenation with / to ensure platform-correct path separators
Follow established patterns when importing new packages, using helper imports rather than direct imports (e.g., use import { generateUuid } from '../platform/common/uuid' instead of importing uuid directly)
Add blank lines after const groups and before return statements for readability
Separate third-party and local file imports

Files:

• src/kernels/deepnote/deepnoteLspClientManager.node.ts

🔇 Additional comments (6)

src/kernels/deepnote/deepnoteLspClientManager.node.ts (2)

384-386: NODE_PATH setup looks correct.

Conditional injection handles the undefined case well.

---

388-398: Server options correctly propagate NODE_PATH.

Both run and debug transports receive the extended environment.

build/esbuild/build.ts (4)

486-487: Correct integration into build pipeline.

Desktop-only build step follows existing patterns.

---

557-579: Bundle configuration looks reasonable.

Externalized packages match what's installed in sql-lsp-modules. Consider enabling sourcemap for debugging production issues.

---

590-600: The original review comment is incorrect. These hardcoded dependency versions are intentional and serve a specific purpose: they define an isolated dependency tree for the sql-language-server artifact (sql-lsp-modules), which is separate from the main extension. These packages do not appear in the main package.json because they are only needed during the build process for this secondary component. Reading versions from the main package.json would not be appropriate here, as it would create the wrong dependency tree. The current implementation is correct.

Likely an incorrect or invalid review comment.

---

609-609: This review comment is incorrect. sqlite3 is not a dependency in this project.

The code installs sql-lsp dependencies in an isolated directory with --ignore-scripts. While sqlite3 packages do require postinstall scripts for native bindings, sqlite3 is not present in this project's dependency tree. The --ignore-scripts flag may affect sql-lsp's own dependencies, but not sqlite3.

Likely an incorrect or invalid review comment.

[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 8b28cf0 and 1380b01.

📒 Files selected for processing (2)

• build/esbuild/build.ts (2 hunks)
• src/kernels/deepnote/deepnoteLspClientManager.node.ts (5 hunks)

🧰 Additional context used

📓 Path-based instructions (5)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Use l10n.t() for all user-facing strings in TypeScript files
Use typed error classes from src/platform/errors/ instead of generic errors
Use ILogger service instead of console.log for logging
Preserve error details in error messages while scrubbing personally identifiable information (PII)
Prefer async/await over promise chains
Handle cancellation with CancellationToken

Order method, fields and properties first by accessibility (public/private/protected) and then by alphabetical order

Files:

• build/esbuild/build.ts
• src/kernels/deepnote/deepnoteLspClientManager.node.ts

**/*.ts

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

**/*.ts: ALWAYS check the 'Core - Build' task output for TypeScript compilation errors before running any script or declaring work complete
ALWAYS run npm run format-fix before committing changes to ensure proper code formatting
FIX all TypeScript compilation errors before moving forward with development
Use npm run format-fix to auto-fix TypeScript formatting issues before committing
Use npm run lint to check for linter issues in TypeScript files and attempt to fix before committing

Files:

• build/esbuild/build.ts
• src/kernels/deepnote/deepnoteLspClientManager.node.ts

**/*.node.ts

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Platform Implementation - Desktop: Use .node.ts files for full file system access and Python environments

Files:

• src/kernels/deepnote/deepnoteLspClientManager.node.ts

src/kernels/**/*.ts

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

src/kernels/**/*.ts: Respect CancellationToken in all async operations across kernel execution, session communication, and finder operations
Implement proper error handling in files using custom error types from errors/ directory (KernelDeadError, KernelConnectionTimeoutError, KernelInterruptTimeoutError, JupyterInvalidKernelError, KernelDependencyError)

Files:

• src/kernels/deepnote/deepnoteLspClientManager.node.ts

src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{ts,tsx}: Don't add the Microsoft copyright header to new files
Use Uri.joinPath() for constructing file paths instead of string concatenation with / to ensure platform-correct path separators
Follow established patterns when importing new packages, using helper imports rather than direct imports (e.g., use import { generateUuid } from '../platform/common/uuid' instead of importing uuid directly)
Add blank lines after const groups and before return statements for readability
Separate third-party and local file imports

Files:

• src/kernels/deepnote/deepnoteLspClientManager.node.ts

🔇 Additional comments (4)

build/esbuild/build.ts (1)

480-488: Wiring SQL LSP build step into desktop pipeline looks consistent.

Adding buildSqlLanguageServer() alongside the other desktop-only copy/build tasks fits the existing pattern (Promise aggregation via builders.push(...) and Promise.all(builders)), and the gating on bundleConfig !== 'web' keeps it out of pure web builds.

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

385-399: NODE_PATH wiring correctly targets bundled SQL LSP dependencies.

Deriving nodePathEnv from getSqlLspModulesPath() and merging it into run/debug options.env ensures the SQL LSP process can resolve its runtime deps from dist/sql-lsp-modules/node_modules in packaged builds, while gracefully doing nothing in dev when that directory hasn’t been created.

---

544-569: Comment and fallback path now match the .cjs build artifact.

The comment now correctly refers to dist/sqlLanguageServer.cjs, and the fallback path matches the build output. Combined with the initial require.resolve attempt, this keeps both dev (node_modules/@deepnote/sql-language-server/...) and packaged (dist/sqlLanguageServer.cjs) scenarios covered.

---

1-1: Helper for sql-lsp-modules path + fs import is aligned with runtime layout.

Importing fs and adding getSqlLspModulesPath() that:

• Resolves the extension root via vscode.extensions.getExtension(...) with __dirname fallback, and
• Returns undefined when dist/sql-lsp-modules/node_modules is missing

nicely matches the new build step and fixes the earlier “always returns string” type mismatch. The existence check prevents setting a broken NODE_PATH in dev or misconfigured builds.

Also applies to: 571-590