sea-auth-u2m: round-2 fixup — wrap close() in decodeNapiKernelError, raise on U2M+id, case-insensitive validation, preserve all error envelope fields

Addresses devils-advocate-auth-u2m-1 round-1 findings on commit
8e99b40. NF-1 is a HIGH continuation of DA-F1 — the previous fixup
wired `decodeNapiKernelError` at `SeaBackend.openSession` but missed
the second extant napi call site, `SeaSessionBackend.close()`. NF-2
through NF-4 are mediums.

## NF-1 (HIGH) — close() wrapped

`SeaSessionBackend.close()` calls `await this.connection.close()` on
the napi `Connection`. Kernel errors from there (e.g., a delete-
session RPC failure that the kernel chose to surface despite the
fire-and-forget pattern) were reaching JS callers as raw `Error`
with `__databricks_error__:` envelope. Wrapped in try/catch +
`throw decodeNapiKernelError(err)` — same 3-line shape as
openSession.

Per `grep -rn "await this\.native\.\|await this\.connection\." lib/sea/`,
these are the only two napi call sites on `sea-auth-u2m`. Future
napi call sites on sea-execution / sea-results / sea-operation
branches need the same wrap (Phase 2; tracked elsewhere).

## NF-2 (medium) — raise on U2M + oauthClientId

Previously, `oauthClientId` set without `oauthClientSecret` was
silently dropped (kernel uses built-in `databricks-cli`). A user
setting the field clearly expects it honored; silent-drop hid
intent. Flipped to throw HiveDriverError with explicit guidance
("kernel uses 'databricks-cli'; omit for U2M or supply
oauthClientSecret for M2M").

The matching unit test in `tests/unit/sea/auth-u2m.test.ts` flipped
from "drops the supplied oauthClientId" to "rejects oauthClientId
on the U2M path with a clear 'not supported' error".

## NF-3 (medium) — case-insensitive reserved-literal validation

`isBlankOrReserved` previously compared `trimmed === 'undefined'`
and `=== 'null'`, so `'UNDEFINED'`, `'Null'`, `'NULL'`, `'nUlL'`,
etc. slipped through. Changed to `trimmed.toLowerCase()` before
the comparison. New unit case in `auth-edge-cases.test.ts` iterates
five case variants and asserts each rejects.

## NF-4 (medium) — preserve all 7 kernel envelope fields

`decodeNapiKernelError` previously routed only `{code, message,
sqlState}` to the JS error class. The kernel envelope at
`native/sea/src/error.rs:50-89` actually carries 7 fields total
(`code`, `message`, `sqlState`, `errorCode`, `vendorCode`,
`httpStatus`, `retryable`, `queryId`). The remaining 5 were
silently dropped. Thrift parity demand: thrift errors carry these
fields.

Added `attachMetadata(error, meta)` helper that `Object.defineProperty`s
each non-undefined field as a non-enumerable own-property — matches
the way `attachSqlState` works and the way Node attaches `.code` to
system errors. Two new unit tests verify (a) all 5 fields round-trip
through a synthetic envelope, (b) they remain non-enumerable
(absent from `Object.keys(err)`) but accessible via direct property
read.

## Items DEFERRED to Phase 2 (recorded here for the curator)

- NF-5 (envelope versioning): `__databricks_error__:` payload has
  no `version` field. A kernel refactor that renames a field would
  silently break the JS decoder. Phase 2: add `version: 1` to the
  kernel-side serialization, check + fallback on JS side. Not in
  this commit because it requires coordinated kernel-side change.
- NF-6 (U2M e2e harness): `auth-u2m-e2e.test.ts` is fully
  `it.skip` pending the Playwright/Puppeteer harness. Devils-
  advocate noted that port-collision + headless-negative-path
  subsets don't strictly need a browser. Phase 2: enable those
  subsets when the harness lands. Not in this commit because the
  work is mostly harness-wiring rather than test code.

Tests:
- Unit: 79/79 pass (was 74 before this commit: +5 — 1 case-insensitive
  reserved literal sweep, 1 M2M oauthClientSecret reserved-literal
  reject, 2 envelope-metadata preservation, 1 close() decode + 1
  flipped from drop-to-reject which kept the count net same — but
  the OAuthClientId test rewrite is on a different file).
- TypeScript build: clean.
- Native build: unchanged (no Rust changes this commit).

Co-authored-by: Isaac

Author: msrathore-db

lib/sea/SeaAuth.ts

@@ -80,14 +80,14 @@ function prependSlash(str: string): string {
 /**
  * Reject inputs that pass `typeof === 'string' && length > 0` but are
  * structurally useless as credentials: whitespace-only strings, and the
- * literal strings `'undefined'` / `'null'` that buggy shell exports
- * (e.g. `export FOO="$UNSET_VAR"`) produce. Surfacing these here means
- * an OAuth flow's `invalid_client` from the workspace is always a real
- * credential mismatch, never a malformed-input passthrough.
+ * literal strings `'undefined'` / `'null'` (case-insensitive) that buggy
+ * shell exports (e.g. `export FOO="$UNSET_VAR"`) produce. Surfacing
+ * these here means an OAuth flow's `invalid_client` from the workspace
+ * is always a real credential mismatch, never a malformed-input passthrough.
  */
 function isBlankOrReserved(s: string): boolean {
-  const trimmed = s.trim();
-  return trimmed.length === 0 || trimmed === 'undefined' || trimmed === 'null';
+  const normalized = s.trim().toLowerCase();
+  return normalized.length === 0 || normalized === 'undefined' || normalized === 'null';
 }
 
 /**
@@ -182,6 +182,16 @@ export function buildSeaConnectionOptions(options: ConnectionOptions): SeaNative
     // (`DBSQLClient.ts:143`): `oauthClientSecret defined ? M2M : U2M`.
     if (oauth.oauthClientSecret === undefined) {
       // U2M.
+      if (oauth.oauthClientId !== undefined) {
+        // The kernel hardcodes `client_id = "databricks-cli"` for U2M;
+        // there's no JS-side override knob. Silently dropping a
+        // user-supplied id would hide that the kernel ignored it.
+        throw new HiveDriverError(
+          'SEA backend: `oauthClientId` is not supported on the OAuth U2M flow; ' +
+            "the kernel uses the built-in 'databricks-cli' client. " +
+            'Omit `oauthClientId` for U2M, or supply `oauthClientSecret` for the M2M flow.',
+        );
+      }
       if (oauth.persistence !== undefined) {
         throw new HiveDriverError(
           'SEA backend: `persistence` (custom OAuth token store) is not yet wired through ' +

lib/sea/SeaBackend.ts

@@ -122,7 +122,11 @@ export class SeaSessionBackend implements ISessionBackend {
   /* eslint-enable @typescript-eslint/no-unused-vars */
 
   public async close(): Promise<Status> {
-    await this.connection.close();
+    try {
+      await this.connection.close();
+    } catch (err) {
+      throw decodeNapiKernelError(err);
+    }
     return Status.success();
   }
 }

lib/sea/SeaErrorMapping.ts

@@ -149,14 +149,48 @@ export function mapKernelErrorToJsError(kErr: KernelErrorShape): ErrorWithSqlSta
   return attachSqlState(error, sqlstate);
 }
 
+/**
+ * Optional metadata fields the kernel may attach via the
+ * `__databricks_error__:` envelope (per `native/sea/src/error.rs:50-89`).
+ * Attached to the decoded JS error as non-enumerable own-properties so
+ * callers can read them (e.g. `error.httpStatus`) without polluting
+ * `JSON.stringify(error)` output. Matches the way Node attaches
+ * `.code` to system errors and the way `attachSqlState` works above.
+ */
+interface KernelErrorMetadata {
+  errorCode?: string;
+  vendorCode?: number;
+  httpStatus?: number;
+  retryable?: boolean;
+  queryId?: string;
+}
+
+function attachMetadata(error: Error, meta: KernelErrorMetadata): void {
+  for (const key of ['errorCode', 'vendorCode', 'httpStatus', 'retryable', 'queryId'] as const) {
+    const value = meta[key];
+    if (value !== undefined) {
+      Object.defineProperty(error, key, {
+        value,
+        writable: true,
+        enumerable: false,
+        configurable: true,
+      });
+    }
+  }
+}
+
 /**
  * Decode a napi-binding error into the typed JS error class.
  *
  * Two paths:
  *  - Structured kernel error: `Error.message` starts with
  *    {@link ERROR_SENTINEL} followed by a JSON envelope. We strip the
- *    sentinel, parse the JSON, and route through
- *    {@link mapKernelErrorToJsError}.
+ *    sentinel, parse the JSON, route the {@link KernelErrorShape}
+ *    through {@link mapKernelErrorToJsError}, and attach all remaining
+ *    envelope fields (`errorCode`, `vendorCode`, `httpStatus`,
+ *    `retryable`, `queryId`) as non-enumerable own-properties on the
+ *    returned error. Thrift parity demand: thrift errors carry these
+ *    fields, so SEA errors must too.
  *  - Binding-side error (e.g. `napi::Error::new(InvalidArg, "openSession:
  *    \`token\` is required for the requested auth mode")` produced by
  *    the binding's own validation): returned unchanged. These don't
@@ -195,10 +229,28 @@ export function decodeNapiKernelError(err: unknown): Error {
     return err;
   }
 
-  const kErr = parsed as { code: string; message: string; sqlState?: string };
-  return mapKernelErrorToJsError({
+  const kErr = parsed as {
+    code: string;
+    message: string;
+    sqlState?: string;
+    errorCode?: string;
+    vendorCode?: number;
+    httpStatus?: number;
+    retryable?: boolean;
+    queryId?: string;
+  };
+
+  const jsErr = mapKernelErrorToJsError({
     code: kErr.code,
     message: kErr.message,
     sqlstate: kErr.sqlState,
   });
+  attachMetadata(jsErr, {
+    errorCode: kErr.errorCode,
+    vendorCode: kErr.vendorCode,
+    httpStatus: kErr.httpStatus,
+    retryable: kErr.retryable,
+    queryId: kErr.queryId,
+  });
+  return jsErr;
 }

tests/unit/sea/auth-edge-cases.test.ts

@@ -61,6 +61,36 @@ describe('SeaAuth — edge cases (input validation + ambiguity)', () => {
       );
     });
 
+    it('rejects mixed-case "UNDEFINED" / "Null" / "NULL" as PAT (case-insensitive)', () => {
+      for (const reserved of ['UNDEFINED', 'Undefined', 'Null', 'NULL', 'nUlL']) {
+        const opts: ConnectionOptions = {
+          host: 'example.cloud.databricks.com',
+          path: '/sql/1.0/warehouses/abc',
+          token: reserved,
+        };
+
+        expect(() => buildSeaConnectionOptions(opts), `for token=${reserved}`).to.throw(
+          AuthenticationError,
+          /non-empty PAT/,
+        );
+      }
+    });
+
+    it('rejects mixed-case reserved literals on oauthClientSecret too', () => {
+      const opts: ConnectionOptions = {
+        host: 'example.cloud.databricks.com',
+        path: '/sql/1.0/warehouses/abc',
+        authType: 'databricks-oauth',
+        oauthClientId: 'client-uuid',
+        oauthClientSecret: 'NULL',
+      };
+
+      expect(() => buildSeaConnectionOptions(opts)).to.throw(
+        AuthenticationError,
+        /oauthClientSecret.*non-empty non-whitespace/,
+      );
+    });
+
     it('rejects whitespace-only oauthClientId on M2M', () => {
       const opts: ConnectionOptions = {
         host: 'example.cloud.databricks.com',
@@ -339,4 +369,91 @@ describe('SeaBackend — kernel error envelope decoding (DA-F1)', () => {
     expect(caught).to.be.instanceOf(Error);
     expect((caught as Error).message).to.contain('not valid json');
   });
+
+  // NF-4: preserve all 7 kernel envelope fields on the decoded JS error
+  // as non-enumerable own-properties so callers can read them without
+  // polluting JSON.stringify(error).
+  it('preserves errorCode + vendorCode + httpStatus + retryable + queryId on the decoded error', async () => {
+    const binding = bindingRejectingWith(
+      '{"code":"Unavailable","message":"upstream timed out",' +
+        '"sqlState":"08006","errorCode":"UPSTREAM_TIMEOUT","vendorCode":1234,' +
+        '"httpStatus":503,"retryable":true,"queryId":"query-abc-123"}',
+    );
+    const backend = new SeaBackend(binding);
+    await backend.connect(validConnectArgs);
+
+    let caught: unknown;
+    try {
+      await backend.openSession({});
+    } catch (e) {
+      caught = e;
+    }
+    const err = caught as {
+      sqlState?: string;
+      errorCode?: string;
+      vendorCode?: number;
+      httpStatus?: number;
+      retryable?: boolean;
+      queryId?: string;
+    };
+    expect(err.sqlState).to.equal('08006');
+    expect(err.errorCode).to.equal('UPSTREAM_TIMEOUT');
+    expect(err.vendorCode).to.equal(1234);
+    expect(err.httpStatus).to.equal(503);
+    expect(err.retryable).to.equal(true);
+    expect(err.queryId).to.equal('query-abc-123');
+  });
+
+  it('keeps the metadata properties non-enumerable (matches sqlState pattern)', async () => {
+    const binding = bindingRejectingWith(
+      '{"code":"NetworkError","message":"x","httpStatus":502}',
+    );
+    const backend = new SeaBackend(binding);
+    await backend.connect(validConnectArgs);
+
+    let caught: unknown;
+    try {
+      await backend.openSession({});
+    } catch (e) {
+      caught = e;
+    }
+    expect(Object.keys(caught as object)).to.not.include('httpStatus');
+    // But direct access still works.
+    expect((caught as { httpStatus?: number }).httpStatus).to.equal(502);
+  });
+
+  // NF-1: SeaSessionBackend.close() must wrap the napi call too.
+  it('SeaSessionBackend.close() decodes kernel-error envelopes from native.close()', async () => {
+    const { binding } = makeFakeBinding();
+    // Make openSession return a fake Connection whose close() throws
+    // a kernel-shaped envelope.
+    const failingClose = {
+      async executeStatement() {
+        throw new Error('unused');
+      },
+      async close() {
+        throw new Error(
+          '__databricks_error__:{"code":"Internal","message":"server-side close failed"}',
+        );
+      },
+    };
+    binding.openSession = (async () => failingClose as unknown) as typeof binding.openSession;
+
+    const backend = new SeaBackend(binding);
+    await backend.connect(validConnectArgs);
+    const session = await backend.openSession({});
+
+    let caught: unknown;
+    try {
+      await session.close();
+    } catch (e) {
+      caught = e;
+    }
+    // Before the NF-1 fix, this would surface as a raw Error whose
+    // message starts with `__databricks_error__:`. After the fix, the
+    // sentinel is stripped and the typed class is dispatched.
+    expect(caught).to.be.instanceOf(HiveDriverError);
+    expect((caught as Error).message).to.equal('server-side close failed');
+    expect((caught as Error).message).to.not.contain('__databricks_error__');
+  });
 });

tests/unit/sea/auth-u2m.test.ts

@@ -37,25 +37,23 @@ describe('SeaAuth + SeaBackend — OAuth U2M auth flow', () => {
       });
     });
 
-    it('drops the supplied oauthClientId on the U2M path (kernel uses its own default)', () => {
-      // The thrift parity story: thrift's getClientId() falls back to
-      // `databricks-cli` when undefined. Here we tell the kernel to do
-      // the same via `client_id: None`. If a user supplies a clientId
-      // alongside no secret, we treat that as U2M and use kernel default
-      // — explicitly NOT propagating the supplied id, because the kernel
-      // surface for U2M client_id is None-or-Some-with-no-default-rewrite,
-      // and exposing the override here is out-of-scope-for-this-task.
+    it('rejects oauthClientId on the U2M path with a clear "not supported" error', () => {
+      // The kernel hardcodes `client_id = "databricks-cli"` for U2M; there's
+      // no JS-side override knob. Silently dropping a user-supplied id would
+      // hide that the kernel ignored it, so we surface the limitation
+      // explicitly. Earlier revisions of this code silently dropped — flipped
+      // to raise based on devils-advocate-auth-u2m-1 round-1 (NF-2).
       const opts: ConnectionOptions = {
         host: 'example.cloud.databricks.com',
         path: '/sql/1.0/warehouses/abc',
         authType: 'databricks-oauth',
         oauthClientId: 'custom-client',
       };
 
-      const native = buildSeaConnectionOptions(opts);
-      expect(native.authMode).to.equal('OAuthU2m');
-      // Custom clientId is intentionally not forwarded — see comment above.
-      expect(native).to.not.have.property('oauthClientId');
+      expect(() => buildSeaConnectionOptions(opts)).to.throw(
+        HiveDriverError,
+        /oauthClientId.*not supported on the OAuth U2M flow/,
+      );
     });
 
     it('prepends `/` to the path on the U2M branch too', () => {