sea-abstraction: address full-review findings (F1-F17 except F5)

Round-N fixes from the 9-reviewer pre-review. Public IOperation/DBSQLOperation
surface preserved byte-identical; backend interfaces (IBackend / ISessionBackend
/ IOperationBackend) made fully neutral so both Thrift and SEA can implement
the same contract.

F1 — neutral DTOs at IOperationBackend with Thrift-shape preservation on the
  public facade (adapter pattern):
- lib/contracts/OperationStatus.ts (new) — neutral OperationStatus + OperationState
  enum mirroring databricks-sql-python's CommandState and kernel pyo3's
  StatementStatus taxonomy.
- lib/contracts/ResultMetadata.ts (new) — neutral ResultMetadata + ResultFormat
  enum mirroring the three TSparkRowSetType cases.
- IOperationBackend.status()/getResultMetadata() return the neutral DTOs.
- ThriftOperationBackend.status() adapts at the boundary via adaptOperationStatus
  / adaptResultMetadata; module-level helpers thriftStateToOperationState and
  thriftRowSetTypeToResultFormat do the enum maps.
- ThriftOperationBackend exposes thriftStatusResponse() and
  thriftResultMetadataResponse() as public Thrift-only accessors used by the
  facade's zero-loss fast path (kept for internal state-machine + result-handler
  dispatch as well).
- lib/utils/thriftWireSynthesis.ts (new) — synthesizeThriftStatus and
  synthesizeThriftResultSetMetadata: convert neutral DTOs back to Thrift wire
  shape for the non-Thrift backend path. Lossy on Thrift-only fields
  (taskStatus, numModifiedRows, cacheLookupResult, etc.).
- DBSQLOperation.status() and getMetadata() preserved Thrift return shape:
  Thrift backend path returns the real wire response (zero loss); non-Thrift
  backend path synthesizes via the new helpers.
- DBSQLOperation.getResultMetadata() — new additive neutral accessor on
  IOperation; DBSQLSession.handleStagingOperation uses it instead of the
  deprecated Thrift-shaped getMetadata().

F2 — IBackend.connect() is now zero-arg. Backend reads everything it needs
  from IClientContext / constructor; matches Python connector's pattern of
  passing session_configuration via constructor not method-arg.

F3 — Restore the 'Server protocol version' debug log dropped by the original
  PR-378 refactor. Re-added to ThriftSessionBackend.constructor with the
  LogLevel.debug + IClientContext.getLogger() pattern; matches the pre-refactor
  log site at main:lib/DBSQLSession.ts:175.

F4 + F11 + F14 — SeaBackend stub safety:
- close() is a no-op so DBSQLClient.close()'s state-clearing block can finish
  even after a useSEA: true connect() failure.
- connect() and openSession() throw HiveDriverError instead of generic Error,
  matching the rest of the codebase.
- connect(options: ConnectionOptions) and openSession(request: OpenSessionRequest)
  declare their parameters (with @typescript-eslint/no-unused-vars disable)
  so IDE autocomplete prompts the M1 SEA implementer.

F6 + F7 + F9 + F10 — JSDoc on backend interfaces:
- IBackend: connect/openSession/close docstrings; close() doc explicitly
  states transport-layer cleanup is owned by DBSQLClient.
- ISessionBackend: copy IDBSQLSession's per-method one-liner JSDoc.
- IOperationBackend: doc hasResultSet (readonly external; mutates internally),
  waitUntilReady (MUST throw OperationStateError on terminal non-success).

F8 — tests/unit/sea/SeaBackend.test.ts (new) locks in the stub contract:
  connect() rejects HiveDriverError, openSession() rejects HiveDriverError,
  close() resolves no-op. ~30 LOC.

F12 — Drop legacy { handle, ... } ctor branch from DBSQLOperation and
  DBSQLSession:
- Facades accept only { backend, context }.
- DBSQLSession no longer imports ThriftSessionBackend at all.
- DBSQLOperation imports ThriftOperationBackend solely for the F1 typed
  downcast (zero-loss Thrift fast path); this is a deliberate, scoped
  coupling tied to the back-compat decision.
- tests/unit/.stubs/createSessionForTest.ts and createOperationForTest.ts
  (new) wrap the legacy shape; all 48 + 54 test sites mechanically migrated.

F15 — ThriftOperationBackend.waitUntilReady uses imported WaitUntilReadyOptions
  type instead of an anonymous inline shape.

F16 — useSEA flag moved out of public ConnectionOptions:
- Removed useSEA?: boolean from the exported lib/contracts/IDBSQLClient.ts
  ConnectionOptions; no longer ships in the public .d.ts.
- lib/contracts/InternalConnectionOptions.ts (new) declares the flag as a
  non-exported internal extension; DBSQLClient.connect() reads via a typed
  cast. Mirrors Python's kwargs.get('use_sea', False) pattern at
  databricks-sql-python/src/databricks/sql/session.py:111.

F17 — Missing return; after case 'timeout' in forwardConnectionEvent so a
  future fifth case doesn't silently fall through. The trailing return; in
  the last case triggers no-useless-return — quieted with a localized
  eslint-disable-next-line + intent comment.

F5 — deferred per owner instruction (test-only as any cast tightening).

Verification:
- yarn lint clean (3 pre-existing warnings in tests/e2e/protocol_versions.test.ts).
- yarn build clean.
- tsc --noEmit -p tsconfig.json clean (apart from pre-existing
  examples/tokenFederation/* import errors that exist on main).
- Runtime smoke test of SeaBackend stub + Thrift-wire synthesis round-trip
  passes 5/5 assertions.

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

Author: msrathore-db

lib/DBSQLClient.ts

@@ -15,6 +15,7 @@ import IConnectionOptions from './connection/contracts/IConnectionOptions';
 import HiveDriverError from './errors/HiveDriverError';
 import { buildUserAgentString } from './utils';
 import IBackend from './contracts/IBackend';
+import { InternalConnectionOptions } from './contracts/InternalConnectionOptions';
 import ThriftBackend from './thrift-backend/ThriftBackend';
 import SeaBackend from './sea/SeaBackend';
 import PlainHttpAuthentication from './connection/auth/PlainHttpAuthentication';
@@ -237,7 +238,11 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I
 
     this.connectionProvider = this.createConnectionProvider(options);
 
-    this.backend = options.useSEA
+    // M0: `useSEA` is consumed via a non-exported internal-options cast so it
+    // doesn't ship in the public `.d.ts`. Mirrors Python's `kwargs.get("use_sea")`
+    // pattern (see databricks-sql-python/src/databricks/sql/session.py).
+    const internalOptions = options as ConnectionOptions & InternalConnectionOptions;
+    this.backend = internalOptions.useSEA
       ? new SeaBackend()
       : new ThriftBackend({
           context: this,
@@ -272,6 +277,10 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I
       case 'timeout':
         this.logger.log(LogLevel.debug, 'Connection timed out.');
         this.emit('timeout');
+        // Explicit return mirrors the other cases and protects against
+        // fall-through if a new event is added below.
+        // eslint-disable-next-line no-useless-return
+        return;
       // no default
     }
   }

lib/DBSQLOperation.ts

@@ -11,29 +11,23 @@ import IOperation, {
 } from './contracts/IOperation';
 import {
   TGetOperationStatusResp,
-  TOperationHandle,
-  TTableSchema,
-  TSparkDirectResults,
   TGetResultSetMetadataResp,
+  TTableSchema,
 } from '../thrift/TCLIService_types';
 import Status from './dto/Status';
 import { LogLevel } from './contracts/IDBSQLLogger';
 import OperationStateError, { OperationStateErrorCode } from './errors/OperationStateError';
 import { OperationChunksIterator, OperationRowsIterator } from './utils/OperationIterator';
 import IClientContext from './contracts/IClientContext';
 import IOperationBackend from './contracts/IOperationBackend';
+import { ResultMetadata } from './contracts/ResultMetadata';
 import ThriftOperationBackend from './thrift-backend/ThriftOperationBackend';
+import { synthesizeThriftStatus, synthesizeThriftResultSetMetadata } from './utils/thriftWireSynthesis';
 
-type DBSQLOperationConstructorOptions =
-  | {
-      handle: TOperationHandle;
-      directResults?: TSparkDirectResults;
-      context: IClientContext;
-    }
-  | {
-      backend: IOperationBackend;
-      context: IClientContext;
-    };
+interface DBSQLOperationConstructorOptions {
+  backend: IOperationBackend;
+  context: IClientContext;
+}
 
 export default class DBSQLOperation implements IOperation {
   private readonly context: IClientContext;
@@ -48,14 +42,7 @@ export default class DBSQLOperation implements IOperation {
 
   constructor(options: DBSQLOperationConstructorOptions) {
     this.context = options.context;
-    this.backend =
-      'backend' in options
-        ? options.backend
-        : new ThriftOperationBackend({
-            handle: options.handle,
-            directResults: options.directResults,
-            context: options.context,
-          });
+    this.backend = options.backend;
     this.context.getLogger().log(LogLevel.debug, `Operation created with id: ${this.id}`);
   }
 
@@ -144,14 +131,27 @@ export default class DBSQLOperation implements IOperation {
   }
 
   /**
-   * Requests operation status
+   * Requests operation status. Returns the Thrift wire response for
+   * back-compat with existing user code. On the Thrift backend the response
+   * is returned verbatim; on any other backend (e.g. SEA) the response is
+   * synthesized from the neutral {@link IOperationBackend.status} result,
+   * with Thrift-only fields (`taskStatus`, `numModifiedRows`, etc.) left
+   * undefined.
+   *
    * @param progress
    * @throws {StatusError}
    */
   public async status(progress: boolean = false): Promise<TGetOperationStatusResp> {
     await this.failIfClosed();
     this.context.getLogger().log(LogLevel.debug, `Fetching status for operation with id: ${this.id}`);
-    return this.backend.status(progress);
+    if (this.backend instanceof ThriftOperationBackend) {
+      // Zero-loss path: the Thrift backend has the wire response on hand.
+      return this.backend.thriftStatusResponse(progress);
+    }
+    // Non-Thrift backend: synthesize the Thrift-shaped response from the
+    // neutral OperationStatus DTO.
+    const status = await this.backend.status(progress);
+    return synthesizeThriftStatus(status);
   }
 
   /**
@@ -213,12 +213,31 @@ export default class DBSQLOperation implements IOperation {
     return metadata.schema ?? null;
   }
 
-  public async getMetadata(): Promise<TGetResultSetMetadataResp> {
+  public async getResultMetadata(): Promise<ResultMetadata> {
     await this.failIfClosed();
     await this.waitUntilReadyThroughBackend();
     return this.backend.getResultMetadata();
   }
 
+  /**
+   * Fetch result-set metadata as the Thrift wire response. Kept for
+   * back-compat with existing user code. On the Thrift backend the wire
+   * response is returned verbatim; on any other backend the response is
+   * synthesized from the neutral {@link ResultMetadata}, with Thrift-only
+   * fields (`cacheLookupResult`, `uncompressedBytes`, `compressedBytes`,
+   * `status`) left undefined / defaulted.
+   *
+   * Prefer {@link DBSQLOperation.getResultMetadata} in new code.
+   */
+  public async getMetadata(): Promise<TGetResultSetMetadataResp> {
+    await this.failIfClosed();
+    await this.waitUntilReadyThroughBackend();
+    if (this.backend instanceof ThriftOperationBackend) {
+      return this.backend.thriftResultMetadataResponse();
+    }
+    return synthesizeThriftResultSetMetadata(await this.backend.getResultMetadata());
+  }
+
   private async failIfClosed(): Promise<void> {
     if (this.closed) {
       throw new OperationStateError(OperationStateErrorCode.Closed);

lib/DBSQLSession.ts

@@ -3,7 +3,6 @@ import * as path from 'path';
 import stream from 'node:stream';
 import util from 'node:util';
 import fetch, { HeadersInit } from 'node-fetch';
-import { TSessionHandle, TProtocolVersion } from '../thrift/TCLIService_types';
 import IDBSQLSession, {
   ExecuteStatementOptions,
   TypeInfoRequest,
@@ -27,24 +26,17 @@ import StagingError from './errors/StagingError';
 import IClientContext from './contracts/IClientContext';
 import ISessionBackend from './contracts/ISessionBackend';
 import IOperationBackend from './contracts/IOperationBackend';
-import ThriftSessionBackend from './thrift-backend/ThriftSessionBackend';
 
 // Explicitly promisify a callback-style `pipeline` because `node:stream/promises` is not available in Node 14
 const pipeline = util.promisify(stream.pipeline);
 
 // Re-export for back-compat with existing imports.
 export { numberToInt64 } from './thrift-backend/ThriftSessionBackend';
 
-type DBSQLSessionConstructorOptions =
-  | {
-      handle: TSessionHandle;
-      context: IClientContext;
-      serverProtocolVersion?: TProtocolVersion;
-    }
-  | {
-      backend: ISessionBackend;
-      context: IClientContext;
-    };
+interface DBSQLSessionConstructorOptions {
+  backend: ISessionBackend;
+  context: IClientContext;
+}
 
 export default class DBSQLSession implements IDBSQLSession {
   private readonly context: IClientContext;
@@ -59,14 +51,7 @@ export default class DBSQLSession implements IDBSQLSession {
 
   constructor(options: DBSQLSessionConstructorOptions) {
     this.context = options.context;
-    this.backend =
-      'backend' in options
-        ? options.backend
-        : new ThriftSessionBackend({
-            handle: options.handle,
-            context: options.context,
-            serverProtocolVersion: options.serverProtocolVersion,
-          });
+    this.backend = options.backend;
     this.context.getLogger().log(LogLevel.debug, `Session created with id: ${this.id}`);
   }
 
@@ -106,7 +91,7 @@ export default class DBSQLSession implements IDBSQLSession {
 
     // Staging detection: only run when stagingAllowedLocalPath is provided.
     if (options.stagingAllowedLocalPath !== undefined) {
-      const metadata = await operation.getMetadata();
+      const metadata = await operation.getResultMetadata();
       if (metadata.isStagingOperation) {
         const allowedLocalPath = Array.isArray(options.stagingAllowedLocalPath)
           ? options.stagingAllowedLocalPath

lib/contracts/IBackend.ts

@@ -7,9 +7,28 @@ import ISessionBackend from './ISessionBackend';
  * re-selected per-call.
  */
 export default interface IBackend {
+  /**
+   * Establish backend-level state before any session is opened. Implementations
+   * consume `options` to build backend-specific connection parameters (e.g. the
+   * SEA backend derives napi-binding `SeaNativeConnectionOptions` from the auth
+   * + host fields here). Transport-layer connection providers are owned by
+   * `DBSQLClient` (via `IClientContext`) and exposed to backends through
+   * constructor injection.
+   */
   connect(options: ConnectionOptions): Promise<void>;
 
+  /**
+   * Open a session. Returned `ISessionBackend` is owned by the caller
+   * and torn down via its own `close()`.
+   */
   openSession(request: OpenSessionRequest): Promise<ISessionBackend>;
 
+  /**
+   * Backend-level teardown. Transport-layer cleanup (connection provider,
+   * thrift client, auth provider) is owned by `DBSQLClient` and runs
+   * after this returns. Implementations release backend-internal resources
+   * here, and MUST be safe to call on a partially-initialized backend
+   * (i.e. after a failed `connect()`).
+   */
   close(): Promise<void>;
 }

lib/contracts/IDBSQLClient.ts

@@ -54,12 +54,6 @@ export type ConnectionOptions = {
   socketTimeout?: number;
   proxy?: ProxyOptions;
   enableMetricViewMetadata?: boolean;
-  /**
-   * Opt-in flag to dispatch through the Statement Execution API (SEA) backend
-   * instead of the default Thrift backend. Defaults to `false`.
-   * @internal Not stable; M0 stub only.
-   */
-  useSEA?: boolean;
 } & AuthOptions;
 
 export interface OpenSessionRequest {

lib/contracts/IOperation.ts

@@ -1,6 +1,7 @@
 import { Readable, ReadableOptions } from 'node:stream';
 import { TGetOperationStatusResp, TTableSchema } from '../../thrift/TCLIService_types';
 import Status from '../dto/Status';
+import { ResultMetadata } from './ResultMetadata';
 
 export type OperationStatusCallback = (progress: TGetOperationStatusResp) => unknown;
 
@@ -59,7 +60,10 @@ export default interface IOperation {
   fetchAll(options?: FetchOptions): Promise<Array<object>>;
 
   /**
-   * Request status of operation
+   * Request status of operation. Returns the Thrift wire response for
+   * back-compat. New code should prefer {@link IOperation.getResultMetadata}
+   * for metadata and may consume the neutral `IOperationBackend.status` via
+   * a typed downcast when implementing alternative backends.
    *
    * @param progress
    */
@@ -90,6 +94,12 @@ export default interface IOperation {
    */
   getSchema(options?: GetSchemaOptions): Promise<TTableSchema | null>;
 
+  /**
+   * Fetch result-set metadata in the backend-neutral `ResultMetadata` shape.
+   * Prefer this over the Thrift-shaped surface for new code.
+   */
+  getResultMetadata(): Promise<ResultMetadata>;
+
   iterateChunks(options?: IteratorOptions): IOperationChunksIterator;
 
   iterateRows(options?: IteratorOptions): IOperationRowsIterator;

lib/contracts/IOperationBackend.ts

@@ -1,27 +1,55 @@
-import { TGetOperationStatusResp, TGetResultSetMetadataResp } from '../../thrift/TCLIService_types';
 import Status from '../dto/Status';
 import { WaitUntilReadyOptions } from './IOperation';
+import { OperationStatus } from './OperationStatus';
+import { ResultMetadata } from './ResultMetadata';
 
 /**
  * What a `DBSQLOperation` needs from its backend. Returned by
  * `ISessionBackend.executeStatement` and the metadata methods.
  */
 export default interface IOperationBackend {
+  /** Operation identifier. */
   readonly id: string;
 
+  /**
+   * Whether this operation has a result set. Initial value may be derived
+   * from the create-operation response; implementations MUST refresh it
+   * from terminal status responses (the Thrift impl updates
+   * `operationHandle.hasResultSet` inside `processOperationStatusResponse`).
+   * `readonly` here means external callers cannot reassign the property —
+   * not that the underlying value is fixed at construction time.
+   */
   readonly hasResultSet: boolean;
 
+  /** Fetch the next chunk of result rows. */
   fetchChunk(options: { limit: number; disableBuffering?: boolean }): Promise<Array<object>>;
 
+  /** Whether more rows are available beyond what has been fetched. */
   hasMore(): Promise<boolean>;
 
+  /**
+   * Poll the backend until the operation reaches a terminal state.
+   *
+   * MUST throw `OperationStateError` (with one of `OperationStateErrorCode.{Canceled,
+   * Closed, Error, Timeout, Unknown}`) on terminal non-success states. The
+   * `DBSQLOperation` facade depends on `Canceled` and `Closed` codes to mirror
+   * the operation into its closed/cancelled flags; future implementations must
+   * use the same error type for the facade to stay in sync.
+   */
   waitUntilReady(options?: WaitUntilReadyOptions): Promise<void>;
 
-  status(progress: boolean): Promise<TGetOperationStatusResp>;
+  /**
+   * Fetch operation status as a neutral `OperationStatus`. Pass `progress: true`
+   * to request that the backend include a progress payload.
+   */
+  status(progress: boolean): Promise<OperationStatus>;
 
-  getResultMetadata(): Promise<TGetResultSetMetadataResp>;
+  /** Fetch result-set metadata (schema, format, lz4 flag, arrow schema, staging flag). */
+  getResultMetadata(): Promise<ResultMetadata>;
 
+  /** Cancel the operation. */
   cancel(): Promise<Status>;
 
+  /** Close the operation. Idempotent. */
   close(): Promise<Status>;
 }

lib/contracts/ISessionBackend.ts

@@ -19,21 +19,42 @@ import InfoValue from '../dto/InfoValue';
  * `IBackend.openSession()`. Lifecycle tied to a single `DBSQLSession`.
  */
 export default interface ISessionBackend {
+  /** Session identifier. */
   readonly id: string;
 
+  /** Returns general information about the data source. */
   getInfo(infoType: number): Promise<InfoValue>;
 
+  /** Executes DDL/DML statements. */
   executeStatement(statement: string, options: ExecuteStatementOptions): Promise<IOperationBackend>;
 
+  /** Information about supported data types. */
   getTypeInfo(request: TypeInfoRequest): Promise<IOperationBackend>;
+
+  /** List of catalogs. */
   getCatalogs(request: CatalogsRequest): Promise<IOperationBackend>;
+
+  /** List of schemas. */
   getSchemas(request: SchemasRequest): Promise<IOperationBackend>;
+
+  /** List of tables. */
   getTables(request: TablesRequest): Promise<IOperationBackend>;
+
+  /** List of supported table types. */
   getTableTypes(request: TableTypesRequest): Promise<IOperationBackend>;
+
+  /** Full column information for a table. */
   getColumns(request: ColumnsRequest): Promise<IOperationBackend>;
+
+  /** Information about a function. */
   getFunctions(request: FunctionsRequest): Promise<IOperationBackend>;
+
+  /** Primary keys of a table. */
   getPrimaryKeys(request: PrimaryKeysRequest): Promise<IOperationBackend>;
+
+  /** Foreign-key relationships between two tables. */
   getCrossReference(request: CrossReferenceRequest): Promise<IOperationBackend>;
 
+  /** Close the session. Idempotent. */
   close(): Promise<Status>;
 }