sea-auth-u2m: round-5 fixup — JSDoc selector contract, defense-in-depth test, message mutation safety, class-pin simplification

- DA4-1 (HIGH): rewrite buildSeaConnectionOptions function-level
  JSDoc to describe the id-keyed flow selector (round-4 NF3-2 fix);
  the block-comment was updated but the public-API JSDoc was missed.
- DA4-2 (MEDIUM): add test for the SeaAuth.ts:201-210
  defense-in-depth U2M+id rejection branch (zero coverage after
  round-4 flipped the three tests that previously exercised it).
- DA4-3a (MEDIUM): wrap err.message mutation on corrupted-envelope
  path in try/catch; fall back to a fresh HiveDriverError if the
  message descriptor is non-writable (defensive for future napi-rs
  versions; mutation preserves napi-side stack on the common path).
- DA4-3b (MEDIUM): delete redundant constructor.name check from
  class-pin test; instanceof AuthenticationError is sufficient
  because instanceof is a one-way subclass check. Fix the comment
  that incorrectly claimed instanceof couldn't distinguish.
- LE4-1 (MEDIUM): add this.name = 'AuthenticationError' constructor
  to the AuthenticationError class so err.name / err.toString()
  identify the subclass (3 lines; doesn't extend to sibling error
  classes in this PR).
- DA4-4 (LOW): drop "reserved for future RetryError mappings" from
  three SeaErrorMapping.ts doc-comments — no kernel ErrorCode maps
  to RetryError and there's no design doc proposing one.
- LE4-2 (LOW): unify the class-pin test to chai's idiomatic
  .to.throw(Class, /regex/) form, matching the rest of the suite.
- LE4-4 (LOW): one-line comment justifying mutate-vs-clone choice
  on the corrupted-envelope path.

Skipped per disposition: LE4-3 (idIsBlank/secretIsBlank symmetry —
LE-4 own recommended leave-as-is).

Deferred (carries over from round-3): NF3-1 kernel sub-classification
(Phase 2 kernel work), NF3-4 e2e kernel-error-code assertion (blocked
on NF3-1), NF3-5 path-dep SHA pin (resolves on kernel publish),
LE3-1..3 SeaErrorMapping decoder polish (Phase 2 bundle).

Co-authored-by: Isaac

Author: msrathore-db

lib/errors/AuthenticationError.ts

@@ -1,3 +1,8 @@
 import HiveDriverError from './HiveDriverError';
 
-export default class AuthenticationError extends HiveDriverError {}
+export default class AuthenticationError extends HiveDriverError {
+  constructor(message?: string) {
+    super(message);
+    this.name = 'AuthenticationError';
+  }
+}

lib/sea/SeaAuth.ts

@@ -104,12 +104,14 @@ export function isBlankOrReserved(s: string): boolean {
  *   - OAuth M2M: `authType: 'databricks-oauth'` + `oauthClientId` +
  *     `oauthClientSecret`. Kernel handles OIDC discovery, client_credentials
  *     exchange, and re-auth on expiry internally.
- *   - OAuth U2M: `authType: 'databricks-oauth'` + NO `oauthClientSecret`.
- *     Kernel runs the PKCE auth-code dance (opens a browser, listens on
- *     localhost:8030, exchanges the code, persists to
- *     `~/.config/databricks-sql-kernel/oauth/{sha256}.json`). The flow
- *     selector matches thrift at `DBSQLClient.ts:143` —
- *     `oauthClientSecret defined ? M2M : U2M`.
+ *   - OAuth U2M: `authType: 'databricks-oauth'` + NO `oauthClientId` and
+ *     NO `oauthClientSecret`. Kernel runs the PKCE auth-code dance (opens
+ *     a browser, listens on localhost:8030, exchanges the code, persists
+ *     to `~/.config/databricks-sql-kernel/oauth/{sha256}.json`). The flow
+ *     selector keys off `oauthClientId` presence: present → M2M, absent →
+ *     U2M. (Round-4 NF3-2 fix; previously secret-keyed — that variant
+ *     routed a typo'd-secret M2M call to the U2M arm and swallowed the
+ *     actionable error.) Mirrors thrift's intent at `DBSQLClient.ts:143`.
  *
  * Out of scope on the OAuth paths (rejected with a clear error):
  *   - `azureTenantId` / `useDatabricksOAuthInAzure` → Microsoft Entra
@@ -188,7 +190,12 @@ export function buildSeaConnectionOptions(options: ConnectionOptions): SeaNative
     // user who set an id but typoed/forgot the secret gets the M2M
     // "secret is required" error instead of a U2M error that hides
     // their actual intent. The U2M arm still defends against an id
-    // sneaking through (e.g. caller bypasses shape inference).
+    // sneaking through: fires only when `oauthClientId` is provided as
+    // a blank-reserved literal (e.g., whitespace, `"null"`, `"undefined"`)
+    // alongside an absent/blank secret — both `idIsBlank` and
+    // `secretIsBlank` are true so U2M wins routing, but the caller's
+    // intent to use U2M with a partially-set id is ambiguous and
+    // rejected explicitly.
     const idIsBlank =
       oauth.oauthClientId === undefined ||
       (typeof oauth.oauthClientId === 'string' && isBlankOrReserved(oauth.oauthClientId));

lib/sea/SeaErrorMapping.ts

@@ -60,8 +60,7 @@ export type KernelErrorCode =
  * `errorCode: enum` field, and `DBSQLOperation.ts:209` switches on it
  * (`err.errorCode === OperationStateErrorCode.Canceled`). Top-level
  * defineProperty would clobber that enum with a kernel string and break
- * cancel/close detection. (`RetryError.errorCode` is the same shape and
- * is reserved here for future kernel→`RetryError` mappings.)
+ * cancel/close detection.
  */
 export interface KernelMetadata {
   errorCode?: string;
@@ -76,7 +75,7 @@ export interface KernelMetadata {
  * exposed at the top level (no collision in the existing driver error
  * tree); the remaining envelope fields live under a `kernelMetadata`
  * namespace to avoid clobbering pre-existing `errorCode` semantics on
- * `OperationStateError` (and, reserved for future use, `RetryError`).
+ * `OperationStateError`.
  */
 export interface ErrorWithSqlState extends Error {
   sqlState?: string;
@@ -211,9 +210,7 @@ function buildKernelMetadata(parsed: Record<string, unknown>): KernelMetadata {
  *    envelope fields under a single non-enumerable `kernelMetadata`
  *    namespace. Namespacing avoids the collision with
  *    `OperationStateError.errorCode` (an enum already switched on at the
- *    JS layer — see `DBSQLOperation.ts:209`). `RetryError.errorCode`
- *    shares the shape and is reserved for future kernel→`RetryError`
- *    mappings.
+ *    JS layer — see `DBSQLOperation.ts:209`).
  *  - 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
@@ -243,8 +240,20 @@ export function decodeNapiKernelError(err: unknown): Error {
     // `__databricks_error__:` prefix; it's a binding/JS-side framing
     // marker, not user-actionable, and leaking it makes the message
     // confusing to operators triaging a malformed kernel response.
-    err.message = jsonStr;
-    return err;
+    //
+    // Mutate in place when possible so the napi-binding's original
+    // stack survives — that stack is the only useful triage signal on
+    // a malformed-envelope path (where did a sentinel-prefixed
+    // non-JSON message come from?). Fall back to a fresh
+    // `HiveDriverError` only if a future napi-rs revision makes
+    // `Error.message` non-writable (no such guarantee today, but the
+    // descriptor contract is implementation-defined).
+    try {
+      err.message = jsonStr;
+      return err;
+    } catch {
+      return new HiveDriverError(jsonStr);
+    }
   }
 
   if (

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

@@ -157,13 +157,17 @@ describe('SeaAuth — edge cases (input validation + ambiguity)', () => {
       );
     });
 
-    // Round-4 NF3-2: pin the exact class — must be `AuthenticationError`,
-    // not the bare `HiveDriverError` superclass. The round-3 NF-N3 fix
-    // swapped this silently by routing M2M-with-empty-secret through the
-    // U2M arm, which raised a plain `HiveDriverError`. Guard against that
-    // regression by pinning the constructor name (since
-    // `AuthenticationError extends HiveDriverError`, `instanceof` alone
-    // can't distinguish the two).
+    // Round-4 NF3-2: pin the exact class against the round-3 NF-N3
+    // regression where M2M-with-empty-secret was routed through the U2M
+    // arm and raised a bare `HiveDriverError`. `instanceof
+    // AuthenticationError` correctly returns `false` for a bare
+    // `HiveDriverError` instance (instanceof is a one-way subclass
+    // check), so the subclass check IS sufficient to catch the
+    // regression. We don't add an `error.name` or `constructor.name`
+    // belt — the former requires `this.name` on the subclass (LE4-1
+    // handles that separately for downstream-consumer benefit, not for
+    // this test), and the latter is bundler-fragile (terser/esbuild
+    // strip class names without `keep_classnames`).
     it('M2M-with-empty-secret throws AuthenticationError, not bare HiveDriverError (class pin)', () => {
       const opts: ConnectionOptions = {
         host: 'example.cloud.databricks.com',
@@ -173,15 +177,31 @@ describe('SeaAuth — edge cases (input validation + ambiguity)', () => {
         oauthClientSecret: '',
       };
 
-      let caught: unknown;
-      try {
-        buildSeaConnectionOptions(opts);
-      } catch (e) {
-        caught = e;
-      }
-      expect(caught).to.be.instanceOf(AuthenticationError);
-      expect((caught as Error).constructor.name).to.equal('AuthenticationError');
-      expect((caught as Error).message).to.match(/oauthClientSecret.*non-empty.*OAuth M2M/);
+      expect(() => buildSeaConnectionOptions(opts)).to.throw(
+        AuthenticationError,
+        /oauthClientSecret.*non-empty.*OAuth M2M/,
+      );
+    });
+
+    // Round-5 DA4-2: the round-3 → round-4 test flips left the U2M-arm
+    // defense-in-depth U2M+id rejection without coverage. It's still
+    // reachable: when `oauthClientId` is a blank-reserved literal
+    // (whitespace, `"null"`, `"undefined"`) AND `oauthClientSecret` is
+    // absent/blank, BOTH `idIsBlank` and `secretIsBlank` are true so
+    // U2M wins routing — but a non-undefined id signals ambiguity that
+    // U2M cannot honor (the kernel hardcodes `databricks-cli`).
+    it('routes a whitespace oauthClientId with no oauthClientSecret to the U2M defense-in-depth rejection', () => {
+      const opts: ConnectionOptions = {
+        host: 'example.cloud.databricks.com',
+        path: '/sql/1.0/warehouses/abc',
+        authType: 'databricks-oauth',
+        oauthClientId: '   ',
+      } as unknown as ConnectionOptions;
+
+      expect(() => buildSeaConnectionOptions(opts)).to.throw(
+        HiveDriverError,
+        /oauthClientId.*not supported on the OAuth U2M flow/,
+      );
     });
   });