fix(sea): F1/F2 fixups — u32 upper-bound + mock test + skip-gate + strict typeId asserts

DA round 1 findings on F1 (max_connections) and F2 (complex types
as Arrow), addressed in a single fixup because both touch the F1
NodeJS branch.

F1 — M1 upper-bound: the JS-layer validation accepted any positive
integer but the napi binding accepts `u32`. A value > 2^32 - 1
would either silently truncate at FFI or throw a cryptic kernel
error. Add an explicit upper-bound check with a clear
`HiveDriverError` message that points at typical pool sizes.
Three new unit tests: accept MAX_U32, reject MAX_U32+1, reject
Number.MAX_SAFE_INTEGER.

F1 — M2 mock test: the existing unit suite verified the
buildSeaConnectionOptions output shape but did not exercise the
end-to-end SeaBackend.connect → openSession → napi.openSession
round-trip. Add two mock-binding tests via the existing
`makeFakeBinding` helper that:
1. Confirm maxConnections is forwarded to the napi openSession call
2. Confirm maxConnections is `undefined` (not 0) when omitted, so
   the napi binding's `Option<u32>` reads None and the kernel
   default of 100 applies.

F2 — H1 skip-gate: the e2e test imported `getSeaNative` at module
top-level, which transitively ran `SeaNativeLoader.ts`'s
module-level `require('../../native/sea/index.js')`. When the
`.node` artifact isn't built (`yarn build:native` not run), that
require throws MODULE_NOT_FOUND BEFORE mocha can invoke `before()`,
crashing test discovery for the whole suite. Fix:
1. Verify the native artifact exists in `before()` and skip with
   a clear "run yarn build:native" message if absent
2. Lazy-require the napi shim via `createRequire(import.meta.url)`
   inside `loadBinding()` so the require lives outside module-load
   path. `createRequire` (vs bare `require`) handles the ESM
   reparse path mocha 11+ takes for ts-node-emitted files
   (MODULE_TYPELESS_PACKAGE_JSON warning).

F2 — M2 strict assertions: prior regex-based assertions
(`/List/i`, `/Map|List/i`, etc.) were too permissive — they would
accept LargeList or FixedSizeList where the kernel must return
plain `List`. Switch to arrow-js `Type` enum comparisons via
`.type.typeId`. Adds:
- Strict typeId equality for c_arr (List), c_map (Map), c_struct
  (Struct), c_arr_struct (List)
- Nested-structural check that c_arr_struct's child is Struct
- Row-count sanity assert (literal SELECT → 1 row)

F2 — live e2e: ran against pecotesting (`DATABRICKS_PECOTESTING_*`)
with the freshly-built napi binary. All assertions passed in 655ms.
The kernel default `complex_types_as_json=false` is verified to
produce native Arrow shapes for ARRAY/MAP/STRUCT and nested
ARRAY<STRUCT> end-to-end.

Co-authored-by: Isaac
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>

Author: msrathore-db

lib/sea/SeaAuth.ts

@@ -173,14 +173,23 @@ export function buildSeaConnectionOptions(options: ConnectionOptions): SeaNative
   };
   // Connection-level pool size — forwarded to the napi binding,
   // ignored when undefined so the kernel default (100) applies.
-  // Validate at the JS layer (positive integer) so a misuse fails
-  // here instead of inside the kernel with a cryptic message.
+  // Validate at the JS layer (positive integer, ≤ 2^32 - 1 since the
+  // napi binding accepts `u32`) so a misuse fails here with a clear
+  // `HiveDriverError` instead of either silently truncating at the
+  // FFI boundary or surfacing a cryptic kernel-side error. Upper-
+  // bound check addresses DA round-1 M1 finding.
+  const MAX_U32 = 0xff_ff_ff_ff; // 4_294_967_295
   if (options.maxConnections !== undefined) {
     if (!Number.isInteger(options.maxConnections) || options.maxConnections < 1) {
       throw new HiveDriverError(
         `SEA backend: \`maxConnections\` must be a positive integer; got ${options.maxConnections}.`,
       );
     }
+    if (options.maxConnections > MAX_U32) {
+      throw new HiveDriverError(
+        `SEA backend: \`maxConnections\` exceeds the napi u32 limit (${MAX_U32}); got ${options.maxConnections}. Typical pool sizes are 10-500.`,
+      );
+    }
     base.maxConnections = options.maxConnections;
   }
 

tests/e2e/sea/complex-types-e2e.test.ts

@@ -34,8 +34,22 @@
  */
 
 import { expect } from 'chai';
-import { tableFromIPC } from 'apache-arrow';
-import { getSeaNative } from '../../../lib/sea/SeaNativeLoader';
+import { tableFromIPC, Type as ArrowType } from 'apache-arrow';
+import * as fs from 'fs';
+import * as path from 'path';
+import { createRequire } from 'module';
+
+// Prerequisites for the live e2e:
+//   1. `DATABRICKS_PECOTESTING_*` env vars set (host, http_path, token)
+//   2. `yarn build:native` has produced `native/sea/index.linux-x64-gnu.node`
+//
+// SeaNativeLoader.ts does a module-level `const native =
+// require('../../native/sea/index.js')` which crashes file evaluation
+// (MODULE_NOT_FOUND) when the `.node` artifact is absent, BEFORE mocha
+// can run the `before()` skip-gate. So we (a) verify both prereqs from
+// the suite's `before()` and skip-and-explain, and (b) defer the
+// `getSeaNative` import to inside the test bodies via a synchronous
+// import-on-demand. DA round-1 H1 fixup.
 
 interface NativeBinding {
   openSession(opts: {
@@ -71,11 +85,46 @@ describe('SEA complex types — native Arrow default', function suite() {
     if (!hostName || !httpPath || !token) {
       // eslint-disable-next-line no-invalid-this
       this.skip();
+      return;
+    }
+    // Verify the native artifact exists before any test in the suite
+    // attempts to import `getSeaNative` from SeaNativeLoader (whose
+    // module-level require would crash the whole file load on
+    // MODULE_NOT_FOUND). Skip-with-message so a developer sees the
+    // actionable instruction instead of a cryptic crash.
+    //
+    // Use `process.cwd()` instead of `__dirname` because mocha may
+    // reparse this file as ESM (MODULE_TYPELESS_PACKAGE_JSON path)
+    // where `__dirname` is undefined. mocha always runs with cwd at
+    // the package root, so this resolves consistently.
+    const nodeArtifact = path.resolve(
+      process.cwd(),
+      'native/sea/index.linux-x64-gnu.node',
+    );
+    if (!fs.existsSync(nodeArtifact)) {
+      // eslint-disable-next-line no-console
+      console.warn(
+        `[sea complex-types e2e] skipping: native binary not built. ` +
+          `Run \`yarn build:native\` (or set DATABRICKS_SQL_KERNEL_REPO + yarn build:native) first.`,
+      );
+      // eslint-disable-next-line no-invalid-this
+      this.skip();
     }
   });
 
+  // Build a `require` function from this module's URL so the call
+  // works under both CJS and ESM reparse paths. mocha 11+ may reparse
+  // ts-node-emitted files as ESM (MODULE_TYPELESS_PACKAGE_JSON), in
+  // which case the bare `require` symbol is undefined.
+  // eslint-disable-next-line @typescript-eslint/naming-convention
+  const requireFromHere = createRequire(import.meta.url);
+
+  function loadBinding(): NativeBinding {
+    return requireFromHere('../../../native/sea/index.js') as NativeBinding;
+  }
+
   it('ARRAY / MAP / STRUCT come back as native Arrow shapes', async () => {
-    const binding = getSeaNative() as unknown as NativeBinding;
+    const binding = loadBinding();
     const connection = await binding.openSession({
       hostName: hostName as string,
       httpPath: httpPath as string,
@@ -109,22 +158,56 @@ describe('SEA complex types — native Arrow default', function suite() {
       expect(structField, 'c_struct field present').to.not.equal(undefined);
       expect(arrStructField, 'c_arr_struct field present').to.not.equal(undefined);
 
-      // Arrow type ids per arrow-js — these are the structural checks
-      // that distinguish "native Arrow" from "JSON Utf8". Arrow type
-      // names are stable across arrow-js minor versions.
-      expect(arrField!.type.toString()).to.match(/List/i, 'c_arr should be List');
-      expect(mapField!.type.toString()).to.match(/Map|List/i, 'c_map should be Map (or List of Struct of key/value)');
-      expect(structField!.type.toString()).to.match(/Struct/i, 'c_struct should be Struct');
-      expect(arrStructField!.type.toString()).to.match(/List/i, 'c_arr_struct should be List of Struct');
-
-      // Sanity-check: NONE of the complex columns should be Utf8 — that
-      // would indicate complex_types_as_json was inadvertently enabled.
+      // Strict typeId checks (DA round-1 M2 fixup — prior regex
+      // assertions matched too permissively, e.g. `/List/i` would
+      // accept LargeList or FixedSizeList too. Arrow `Type` is a
+      // numeric enum from arrow-js; comparing typeId is stable
+      // across arrow-js minor versions and a structural match.
+      //
+      // The server returns ARRAY columns as Arrow `List` (typeId 12),
+      // MAP as Arrow `Map` (typeId 17), STRUCT as Arrow `Struct`
+      // (typeId 13). Nested ARRAY<STRUCT> is `List` whose child is
+      // `Struct`.
+      expect(arrField!.type.typeId).to.equal(
+        ArrowType.List,
+        `c_arr typeId should be List(${ArrowType.List}), got ${arrField!.type.typeId} (${arrField!.type})`,
+      );
+      expect(mapField!.type.typeId).to.equal(
+        ArrowType.Map,
+        `c_map typeId should be Map(${ArrowType.Map}), got ${mapField!.type.typeId} (${mapField!.type})`,
+      );
+      expect(structField!.type.typeId).to.equal(
+        ArrowType.Struct,
+        `c_struct typeId should be Struct(${ArrowType.Struct}), got ${structField!.type.typeId} (${structField!.type})`,
+      );
+      expect(arrStructField!.type.typeId).to.equal(
+        ArrowType.List,
+        `c_arr_struct typeId should be List(${ArrowType.List}), got ${arrStructField!.type.typeId} (${arrStructField!.type})`,
+      );
+
+      // Nested-structural check: c_arr_struct should be List<Struct>.
+      // Drilling into `children[0].type.typeId` catches a regression
+      // where the kernel might wrap a Struct in something else (e.g.
+      // FixedSizeList).
+      const arrStructChildType = arrStructField!.type.children[0].type;
+      expect(arrStructChildType.typeId).to.equal(
+        ArrowType.Struct,
+        `c_arr_struct child typeId should be Struct(${ArrowType.Struct}), got ${arrStructChildType.typeId}`,
+      );
+
+      // Negative assertion: NONE of the complex columns should be Utf8
+      // — that would indicate `complex_types_as_json` was inadvertently
+      // enabled on the kernel side. Using typeId here too rather than
+      // a string regex.
       for (const f of [arrField!, mapField!, structField!, arrStructField!]) {
-        expect(f.type.toString()).to.not.match(
-          /^Utf8$/,
-          `${f.name} must not be a JSON string column`,
+        expect(f.type.typeId).to.not.equal(
+          ArrowType.Utf8,
+          `${f.name} must not be a JSON string column (Utf8 typeId)`,
         );
       }
+
+      // Row-count sanity: SELECT-of-literals yields exactly one row.
+      expect(table.numRows).to.equal(1, 'literal SELECT should yield one row');
     } finally {
       if (statement !== null) {
         try {

tests/unit/sea/auth-max-connections.test.ts

@@ -16,6 +16,8 @@ import { expect } from 'chai';
 import { buildSeaConnectionOptions } from '../../../lib/sea/SeaAuth';
 import { ConnectionOptions } from '../../../lib/contracts/IDBSQLClient';
 import HiveDriverError from '../../../lib/errors/HiveDriverError';
+import SeaBackend from '../../../lib/sea/SeaBackend';
+import { makeFakeBinding } from './_helpers/fakeBinding';
 
 describe('SeaAuth — maxConnections plumbing', () => {
   describe('buildSeaConnectionOptions forwards maxConnections', () => {
@@ -125,5 +127,92 @@ describe('SeaAuth — maxConnections plumbing', () => {
       expect(native.authMode).to.equal('OAuthU2m');
       expect(native.maxConnections).to.equal(25);
     });
+
+    // ─── DA round-1 M1 fixup — upper-bound (u32 ceiling) ───────────
+
+    it('accepts the maximum u32 value (4_294_967_295)', () => {
+      const MAX_U32 = 4_294_967_295;
+      const opts: ConnectionOptions = {
+        host: 'example.cloud.databricks.com',
+        path: '/sql/1.0/warehouses/abc',
+        token: 'dapi-fake-pat',
+        maxConnections: MAX_U32,
+      };
+
+      const native = buildSeaConnectionOptions(opts);
+      expect(native.maxConnections).to.equal(MAX_U32);
+    });
+
+    it('rejects values exceeding the napi u32 ceiling', () => {
+      const opts: ConnectionOptions = {
+        host: 'example.cloud.databricks.com',
+        path: '/sql/1.0/warehouses/abc',
+        token: 'dapi-fake-pat',
+        maxConnections: 4_294_967_296, // 2^32, one over u32 max
+      };
+
+      expect(() => buildSeaConnectionOptions(opts)).to.throw(
+        HiveDriverError,
+        /maxConnections.*exceeds.*u32 limit/,
+      );
+    });
+
+    it('rejects 2^53 - 1 (safe integer ceiling — would silently truncate at FFI)', () => {
+      const opts: ConnectionOptions = {
+        host: 'example.cloud.databricks.com',
+        path: '/sql/1.0/warehouses/abc',
+        token: 'dapi-fake-pat',
+        maxConnections: Number.MAX_SAFE_INTEGER,
+      };
+
+      // The JS layer rejects before the FFI boundary so the napi
+      // binding never sees a value it can't faithfully represent.
+      expect(() => buildSeaConnectionOptions(opts)).to.throw(
+        HiveDriverError,
+        /maxConnections.*exceeds.*u32 limit/,
+      );
+    });
+  });
+
+  // ─── DA round-1 M2 fixup — mock-binding round-trip ──────────────
+
+  describe('SeaBackend forwards maxConnections to the napi openSession call', () => {
+    it('passes the user-supplied maxConnections value through', async () => {
+      const { binding, calls } = makeFakeBinding();
+      const backend = new SeaBackend({ nativeBinding: binding });
+
+      await backend.connect({
+        host: 'example.cloud.databricks.com',
+        path: '/sql/1.0/warehouses/abc',
+        token: 'dapi-fake-pat',
+        maxConnections: 250,
+      });
+      await backend.openSession({});
+
+      const openCall = calls.find((c) => c.method === 'openSession');
+      expect(openCall, 'openSession must be called on the binding').to.not.equal(undefined);
+      const opts = openCall!.args[0] as { maxConnections?: number; authMode?: string };
+      expect(opts.maxConnections).to.equal(250);
+      expect(opts.authMode).to.equal('Pat');
+    });
+
+    it('omits maxConnections from the openSession call when not supplied', async () => {
+      const { binding, calls } = makeFakeBinding();
+      const backend = new SeaBackend({ nativeBinding: binding });
+
+      await backend.connect({
+        host: 'example.cloud.databricks.com',
+        path: '/sql/1.0/warehouses/abc',
+        token: 'dapi-fake-pat',
+      });
+      await backend.openSession({});
+
+      const openCall = calls.find((c) => c.method === 'openSession');
+      expect(openCall).to.not.equal(undefined);
+      const opts = openCall!.args[0] as { maxConnections?: number };
+      // `undefined` (not `0`) — the napi binding's `Option<u32>` reads
+      // this as None and applies the kernel default of 100.
+      expect(opts.maxConnections).to.equal(undefined);
+    });
   });
 });