sea-napi-binding: address review — sql-kernel rename, loader class + DI seam, packaging, tests

Rebuilds native/sea against the merged kernel main (binding renamed
@databricks/sea-native -> @databricks/sql-kernel via kernel #82) and
addresses the /full-review findings on PR #380:

- C1: commit the napi-rs router (native/sea/index.js, un-ignored) + a
  prepack assertion so the publish tarball can never ship without it.
  Companion kernel fix (databricks-sql-kernel#93) corrects the base
  package name so the router require paths resolve.
- C2: drop the @sea-native tsconfig path alias; use a relative import
  so no unresolvable specifier leaks into the emitted .d.ts.
- C3/C11: error hints no longer name a 404-ing package; dlopen hint now
  includes the underlying dlerror string + concrete remediation.
- C4: document M0 = linux-x64-gnu-only scope + npm scope-lock note.
- C5: SeaNativeBinding = typeof import('../../native/sea') (no drift).
- C6: SeaNativeLoader is now a class with an injectable load seam;
  getSeaNative/tryGetSeaNative are thin process-global wrappers.
- C7: re-exports renamed Sea* to avoid colliding with Thrift types.
- C8: version.test.ts fails loud on the linux-x64 CI runner + shape
  checks; new loader.test.ts covers the hint branches, Node gate,
  shape check, and caching via the injected seam.
- C9: Node-version guard fails closed (NaN or < floor).
- C13: e2e smoke uses the shared tests/e2e/utils/config.ts creds.
- C10/C12 are resolved upstream in merged kernel main / deferred to M1.

index.d.ts regenerated from merged main: ExecuteOptions dropped
(catalog/schema/sessionConf are session-level on openSession),
close() awaits DeleteSession, schema() is sync, sessionId/statementId
getters added.

Verified: 11 unit tests; e2e SELECT 1 against a live warehouse, direct
and through mitmproxy (SEA REST: POST /sql/sessions -> POST
/sql/statements -> DELETE /sql/sessions/<id>).

Co-authored-by: Isaac

Author: msrathore-db

.gitattributes

@@ -32,8 +32,9 @@ Dockerfile* text
 .gitattributes export-ignore
 .gitignore     export-ignore
 
-# napi-rs auto-generates this file from the kernel's `napi-binding/napi/`
-# crate; regenerated by `npm run build:native`. Tell git/GitHub it's
-# machine-generated so it collapses in diffs and is excluded from
+# napi-rs auto-generates these files from the kernel's `napi-binding/napi/`
+# crate; regenerated by `npm run build:native`. Tell git/GitHub they're
+# machine-generated so they collapse in diffs and are excluded from
 # blame and language stats.
 native/sea/index.d.ts linguist-generated=true
+native/sea/index.js linguist-generated=true

.gitignore

@@ -12,8 +12,10 @@ dist
 lib/version.ts
 
 # SEA native binding — copied/generated from kernel workspace by `npm run build:native`.
-# The committed contract is `native/sea/index.d.ts` (TypeScript declarations).
-# Everything else under native/sea/ is a build artifact and must not be committed.
-native/sea/index.js
+# The committed contract is `native/sea/index.d.ts` (TypeScript declarations) and
+# `native/sea/index.js` (the napi-rs platform router — small, stable, and required in
+# the publish tarball so a missing build step can't ship a tarball that can't load).
+# The `.node` binaries are large per-platform artifacts and must NOT be committed;
+# in production they arrive via the `@databricks/sql-kernel-<triple>` optional deps.
 native/sea/index.node
 native/sea/index.*.node

.npmignore

@@ -4,7 +4,7 @@
 !thrift/**/*
 
 # SEA napi-rs router shim + TypeScript declarations. The router (index.js)
-# selects the per-platform `.node` artifact from `@databricks/sea-native-*`
+# selects the per-platform `.node` artifact from `@databricks/sql-kernel-*`
 # optionalDependencies (populated when the kernel CI publishes them);
 # the .d.ts is the consumer-facing type contract.
 !native/sea/index.js

lib/sea/SeaNativeLoader.ts

@@ -17,31 +17,42 @@
  *
  * Mirrors the load-failure-tolerant pattern of `lib/utils/lz4.ts`: the
  * `.node` artifact ships via per-platform optional dependencies
- * (`@databricks/sea-native-<triple>`), so its absence must not crash
+ * (`@databricks/sql-kernel-<triple>`), so its absence must not crash
  * a Thrift-only consumer of the driver. Callers that actually need
- * SEA invoke `getSeaNative()`, which throws a structured error if
- * the binding could not be loaded.
+ * SEA construct a {@link SeaNativeLoader} (or use the process-global
+ * {@link getSeaNative}) which throws a structured error if the binding
+ * could not be loaded.
+ *
+ * M0 publishes a single triple (`linux-x64-gnu`); see
+ * `native/sea/README.md` for the supported-platform policy.
  */
 
 import type {
   Connection as NativeConnection,
   Statement as NativeStatement,
-  ConnectionOptions,
-  ExecuteOptions,
-  ArrowBatch,
-  ArrowSchema,
-} from '@sea-native';
-
-export type { ConnectionOptions, ExecuteOptions, ArrowBatch, ArrowSchema };
-export type Connection = NativeConnection;
-export type Statement = NativeStatement;
-
-export interface SeaNativeBinding {
-  version(): string;
-  openSession(options: ConnectionOptions): Promise<NativeConnection>;
-  Connection: typeof NativeConnection;
-  Statement: typeof NativeStatement;
-}
+  ConnectionOptions as NativeConnectionOptions,
+  ArrowBatch as NativeArrowBatch,
+  ArrowSchema as NativeArrowSchema,
+} from '../../native/sea';
+
+// SEA-prefixed re-exports. The kernel-generated `.d.ts` keeps the
+// napi-rs default names (`ConnectionOptions`, `ArrowBatch`, …); we
+// disambiguate on the TS-wrapper side so these never collide with the
+// Thrift-side `ConnectionOptions` (lib/contracts/IDBSQLClient.ts) or
+// `ArrowBatch` (lib/result/utils.ts) when imported elsewhere.
+export type SeaConnectionOptions = NativeConnectionOptions;
+export type SeaArrowBatch = NativeArrowBatch;
+export type SeaArrowSchema = NativeArrowSchema;
+export type SeaConnection = NativeConnection;
+export type SeaStatement = NativeStatement;
+
+/**
+ * The full native binding surface, derived from the generated module
+ * so it can never drift from the `.d.ts` contract: when the kernel
+ * adds or renames a free function / class, this type follows
+ * automatically and `defaultRequire`'s cast stays correct.
+ */
+export type SeaNativeBinding = typeof import('../../native/sea');
 
 const MIN_NODE_MAJOR = 18;
 
@@ -50,68 +61,149 @@ function detectNodeMajor(): number {
   return parseInt(process.version.slice(1), 10);
 }
 
+function platformLabel(): string {
+  return `${process.platform}-${process.arch}`;
+}
+
 function loadFailureHint(err: NodeJS.ErrnoException): string {
-  const platform = `${process.platform}-${process.arch}`;
-  const installHint = `Install the matching optional dependency (e.g. @databricks/sea-native-${platform}).`;
+  const platform = platformLabel();
+  // Do not name a concrete package: the published name uses the napi-rs
+  // triple (e.g. `-linux-x64-gnu` / `-linux-x64-musl` / `-win32-x64-msvc`),
+  // not the bare `${platform}` shown here, so a literal example would
+  // 404. Point at the README's supported-triple list instead.
+  const installHint =
+    'Install the matching @databricks/sql-kernel-* optional dependency for your platform ' +
+    '(see native/sea/README.md for the supported triples; M0 ships linux-x64-gnu only).';
   if (err.code === 'MODULE_NOT_FOUND') {
     return `SEA native binding not installed for platform ${platform} on Node ${process.version}. ${installHint}`;
   }
   if (err.code === 'ERR_DLOPEN_FAILED') {
-    return `SEA native binding present but failed to dlopen on platform ${platform} / Node ${process.version} — likely a libc or Node ABI mismatch. The binding requires Node >=${MIN_NODE_MAJOR}.`;
+    // Surface the underlying dlerror string (e.g. `GLIBC_2.32 not found`)
+    // plus concrete remediation — without it the cause is invisible.
+    return (
+      `SEA native binding present but failed to dlopen on platform ${platform} / Node ${process.version}: ` +
+      `${err.message}. Common causes: glibc/musl mismatch (e.g. Alpine Linux — install the -musl variant), ` +
+      `Node ABI mismatch (try \`rm -rf node_modules && npm install\`), or CPU-architecture mismatch. ` +
+      `The binding requires Node >=${MIN_NODE_MAJOR}.`
+    );
   }
   return `SEA native binding failed to load on platform ${platform} / Node ${process.version}: ${err.message}`;
 }
 
-let cached: SeaNativeBinding | null | undefined;
-let cachedError: Error | undefined;
+/**
+ * Default loader: resolves `native/sea/index.js` (the napi-rs router),
+ * which selects the per-platform `.node`. `.js` is omitted so eslint's
+ * `import/extensions` rule accepts the call.
+ */
+function defaultRequire(): SeaNativeBinding {
+  // eslint-disable-next-line @typescript-eslint/no-var-requires, global-require
+  return require('../../native/sea') as SeaNativeBinding;
+}
 
-function tryLoad(): SeaNativeBinding | undefined {
-  const nodeMajor = detectNodeMajor();
-  if (Number.isFinite(nodeMajor) && nodeMajor < MIN_NODE_MAJOR) {
-    cachedError = new Error(
-      `SEA native binding requires Node >=${MIN_NODE_MAJOR}; running Node ${process.version}. Continue using the Thrift backend on this runtime.`,
+/**
+ * Verify the loaded module exposes the surface the driver depends on.
+ * Catches kernel-side renames at load time rather than letting them
+ * surface as `undefined is not a function` deep in a call path.
+ */
+function assertBindingShape(binding: SeaNativeBinding): void {
+  const missing: string[] = [];
+  if (typeof binding.version !== 'function') missing.push('version');
+  if (typeof binding.openSession !== 'function') missing.push('openSession');
+  if (typeof binding.Connection !== 'function') missing.push('Connection');
+  if (typeof binding.Statement !== 'function') missing.push('Statement');
+  if (missing.length > 0) {
+    throw new Error(
+      `SEA native binding loaded but is missing expected export(s): ${missing.join(', ')}. ` +
+        `The kernel-generated binding and the JS loader are out of sync.`,
     );
-    return undefined;
   }
+}
+
+/**
+ * Loads and caches the SEA native binding. Exposed as a class with an
+ * injectable `load` seam so consumers (e.g. `SeaBackend`) can be unit
+ * tested with a stub binding instead of requiring a real `.node` on the
+ * test machine. Most production code uses the process-global default
+ * via {@link getSeaNative} / {@link tryGetSeaNative}.
+ */
+export class SeaNativeLoader {
+  private cached: SeaNativeBinding | null | undefined;
+
+  private cachedError: Error | undefined;
+
+  constructor(private readonly load: () => SeaNativeBinding = defaultRequire) {}
 
-  try {
-    // The require path resolves to `native/sea/index.js` (the napi-rs
-    // router). `.js` is omitted so eslint's `import/extensions` rule
-    // accepts the call.
-    // eslint-disable-next-line @typescript-eslint/no-var-requires, global-require
-    return require('../../native/sea') as SeaNativeBinding;
-  } catch (err) {
-    if (err instanceof Error && 'code' in err) {
-      cachedError = new Error(loadFailureHint(err as NodeJS.ErrnoException));
+  private tryLoad(): SeaNativeBinding | undefined {
+    const nodeMajor = detectNodeMajor();
+    // Fail closed: if we cannot determine the Node major (NaN) or it is
+    // below the floor, refuse the load and fall back to Thrift.
+    if (!Number.isFinite(nodeMajor) || nodeMajor < MIN_NODE_MAJOR) {
+      this.cachedError = new Error(
+        `SEA native binding requires Node >=${MIN_NODE_MAJOR}; running Node ${process.version}. ` +
+          `Continue using the Thrift backend on this runtime.`,
+      );
+      return undefined;
+    }
+
+    try {
+      const binding = this.load();
+      assertBindingShape(binding);
+      return binding;
+    } catch (err) {
+      if (err instanceof Error && 'code' in err) {
+        this.cachedError = new Error(loadFailureHint(err as NodeJS.ErrnoException));
+      } else if (err instanceof Error) {
+        // Shape-check failure or any other Error — preserve its message.
+        this.cachedError = err;
+      } else {
+        this.cachedError = new Error(`SEA native binding failed to load with non-standard error: ${String(err)}`);
+      }
       return undefined;
     }
-    cachedError = new Error(`SEA native binding failed to load with non-standard error: ${String(err)}`);
-    return undefined;
+  }
+
+  /**
+   * Returns the loaded native binding. Throws a structured error if the
+   * binding is unavailable on this platform / Node version.
+   */
+  get(): SeaNativeBinding {
+    if (this.cached === undefined) {
+      this.cached = this.tryLoad() ?? null;
+    }
+    if (this.cached === null) {
+      throw this.cachedError ?? new Error('SEA native binding unavailable');
+    }
+    return this.cached;
+  }
+
+  /**
+   * Returns the loaded binding or `undefined` if it could not be
+   * loaded. Use this for capability-detection at startup; use
+   * {@link get} at the point where SEA is actually required.
+   */
+  tryGet(): SeaNativeBinding | undefined {
+    if (this.cached === undefined) {
+      this.cached = this.tryLoad() ?? null;
+    }
+    return this.cached ?? undefined;
   }
 }
 
+// Process-global default instance + thin convenience wrappers.
+const defaultLoader = new SeaNativeLoader();
+
 /**
- * Returns the loaded native binding. Throws a structured error if
- * the binding is unavailable on this platform / Node version.
+ * Returns the loaded native binding from the process-global loader.
+ * Throws a structured error if the binding is unavailable.
  */
 export function getSeaNative(): SeaNativeBinding {
-  if (cached === undefined) {
-    cached = tryLoad() ?? null;
-  }
-  if (cached === null) {
-    throw cachedError ?? new Error('SEA native binding unavailable');
-  }
-  return cached;
+  return defaultLoader.get();
 }
 
 /**
- * Returns the loaded binding or `undefined` if it could not be
- * loaded. Use this for capability-detection at startup; use
- * `getSeaNative()` at the point where SEA is actually required.
+ * Returns the loaded binding from the process-global loader, or
+ * `undefined` if it could not be loaded.
  */
 export function tryGetSeaNative(): SeaNativeBinding | undefined {
-  if (cached === undefined) {
-    cached = tryLoad() ?? null;
-  }
-  return cached ?? undefined;
+  return defaultLoader.tryGet();
 }

native/sea/README.md

@@ -1,14 +1,15 @@
 # `native/sea/` — consumer-side directory for the Rust napi binding
 
 **The Rust binding source lives in the kernel repo** at
-`databricks-sql-kernel/napi-binding/napi/`. Building it requires a
-local checkout of that repo — see "Build for local dev" below.
+`databricks-sql-kernel/napi/`. Building it requires a local checkout
+of that repo — see "Build for local dev" below. The published npm
+package is `@databricks/sql-kernel-<triple>`.
 
 ## Workspace topology
 
 The napi crate is a **standalone Cargo workspace** (`[workspace]
-members = ["."]` in `napi-binding/napi/Cargo.toml`), **not** a
-sibling of `pyo3/` in the kernel root workspace.
+members = ["."]` in `napi/Cargo.toml`), **not** a sibling of `pyo3/`
+in the kernel root workspace.
 
 The reason is Cargo feature unification. pyo3 builds the kernel with
 the default `tls-native` feature (system OpenSSL via `native-tls`).
@@ -32,31 +33,55 @@ and reintroduce the same clash. Standalone-workspace is the fix.
 - `index.js` — napi-rs's per-platform router shim. Gitignored;
   populated by `npm run build:native` for local dev. In published
   tarballs it ships alongside the `.d.ts` and `require()`s the
-  right `@databricks/sea-native-<triple>` optional dependency.
+  right `@databricks/sql-kernel-<triple>` optional dependency.
 - `index.*.node` — the actual native binary, one per platform.
   Gitignored. In production these live in the per-triple optional
-  dependencies (`@databricks/sea-native-linux-x64-gnu`, etc.); for
+  dependencies (`@databricks/sql-kernel-linux-x64-gnu`, etc.); for
   local dev `npm run build:native` copies one into this directory.
 
 ## Build for local dev
 
 ```bash
 # From the nodejs repo root:
-export DATABRICKS_SQL_KERNEL_REPO=/path/to/your/databricks-sql-kernel/napi-binding
+export DATABRICKS_SQL_KERNEL_REPO=/path/to/your/databricks-sql-kernel
 npm run build:native           # release build (default)
 BUILD_PROFILE=  npm run build:native  # debug build (empty BUILD_PROFILE drops --release)
 ```
 
-`DATABRICKS_SQL_KERNEL_REPO` is required when your kernel checkout
-isn't at `../../databricks-sql-kernel/napi-binding` relative to the
+`DATABRICKS_SQL_KERNEL_REPO` points at the kernel repo root (the
+directory containing `napi/`) and is required when your kernel
+checkout isn't at `../../databricks-sql-kernel` relative to the
 nodejs repo.
 
 ## Production load path
 
 At release time the kernel's CI publishes
-`@databricks/sea-native-<triple>` npm packages — one per supported
+`@databricks/sql-kernel-<triple>` npm packages — one per supported
 platform — each containing a single `.node` binary. The nodejs
 driver lists them as `optionalDependencies`; npm installs only the
 one matching the consumer's `process.platform` / `process.arch`.
 `native/sea/index.js` (the napi-rs router) then `require()`s the
 installed package at load time.
+
+## Supported platforms (M0)
+
+M0 publishes a **single** triple: **`linux-x64-gnu`** (package
+`@databricks/sql-kernel-linux-x64-gnu`). It is the only entry in the
+driver's `optionalDependencies`.
+
+On every other platform (macOS, Windows, linux-arm64, linux-x64-musl
+/ Alpine, …) the SEA binding is simply absent: `SeaNativeLoader`
+returns `undefined` from `tryGet()` / throws a structured
+`MODULE_NOT_FOUND` hint from `get()`, and the driver continues to use
+the Thrift backend exclusively. This is expected, not a regression —
+additional triples are added to `optionalDependencies` as the kernel
+CI starts publishing them in later milestones.
+
+## Supply-chain note
+
+The unpublished triple names (`@databricks/sql-kernel-darwin-arm64`,
+`…-win32-x64-msvc`, etc.) referenced by the router are **not**
+squat-able: `@databricks` is a Databricks-owned npm scope, and npm
+only allows org members to publish under a scope it owns. A third
+party therefore cannot register `@databricks/sql-kernel-*` and have
+the router autoload it. No placeholder packages are required.