Skip to content

Commit 4f6ccc0

Browse files
committed
sea-auth-u2m: address round-1 M2M review parity — shared fakeBinding helper, doc + loader cleanup
Ports the M2M-side round-1 review fixes (commit 88d7d21 on sea-auth-m2m) onto the U2M worktree so the two branches stay aligned in review quality. The U2M-specific work in 5eba37f is unchanged; this commit is pure cleanup applied across all three SEA-auth test files (PAT / M2M / U2M). - Extracted `makeFakeBinding()` to `tests/unit/sea/_helpers/fakeBinding.ts` and refactored all three auth-*.test.ts files to import it. The U2M-worktree commit had THREE copies of the closure (the third was the cause for the bloat reviewer's "rule of three" call-out that the M2M-worktree fixup was meant to forestall). - Dropped the unused `SeaAuthMode` type alias from `SeaAuth.ts` — zero callers; inlined literals already power the discriminated union. - Tightened `SeaNativeBinding.openSession` parameter type to consume the discriminated `SeaNativeConnectionOptions` union from `SeaAuth.ts`, restoring compile-time per-mode field enforcement at the FFI seam. - Augmented the Rust `AuthMode` doc-comment with the napi-emission explanation (PascalCase verbatim, not kebab-case) plus the cross-reference reminder to extend `open_session()`'s match on every new variant. - Added the const-enum hazard note to `SeaNativeConnectionOptions`' doc-comment, locking in the duplicated-literal pattern as deliberate (importing the napi `const enum AuthMode` breaks `isolatedModules`). - Cleaned up the conditional-type-cast lobotomy in `auth-pat.test.ts` on the token-provider fixture; plain `as any` + eslint-disable. Skipped findings (same justification as M2M-worktree commit): F-3 borderline error-class taxonomy, F-4 cosmetic arg-order, F-5 redundant comment-anchor (compiler already enforces), F-8 null vs undefined paranoia, F-9 mocha named-function style. Tests: - Unit: 55/55 pass (same count as 5eba37f — pure restructure). - Native build: clean (1m04s release profile). - Type-check: clean (tsc --noEmit). Co-authored-by: Isaac
1 parent 5eba37f commit 4f6ccc0

8 files changed

Lines changed: 115 additions & 125 deletions

File tree

lib/sea/SeaAuth.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,6 @@ import { ConnectionOptions } from '../contracts/IDBSQLClient';
1616
import AuthenticationError from '../errors/AuthenticationError';
1717
import HiveDriverError from '../errors/HiveDriverError';
1818

19-
/**
20-
* Auth-mode discriminant value crossing the napi boundary. The string
21-
* literals are what napi-rs emits from the `#[napi(string_enum)] AuthMode`
22-
* enum at `native/sea/src/database.rs` — they MUST match the variant
23-
* names verbatim (`'Pat'`, `'OAuthM2m'`, `'OAuthU2m'`).
24-
*/
25-
export type SeaAuthMode = 'Pat' | 'OAuthM2m' | 'OAuthU2m';
26-
2719
/**
2820
* Default local listener port for the U2M authorization-code callback.
2921
* Hardcoded here so the override of the kernel default (8020) to the
@@ -47,6 +39,15 @@ const U2M_DEFAULT_REDIRECT_PORT = 8030;
4739
* - `'OAuthU2m'` → `oauthRedirectPort` overrides the kernel default;
4840
* everything else (client_id, scopes, callback timeout,
4941
* token_url_override) uses kernel defaults.
42+
*
43+
* The `authMode` string literals MUST match the napi-emitted `AuthMode`
44+
* variant names verbatim (`'Pat'`, `'OAuthM2m'`, `'OAuthU2m'` — napi-rs's
45+
* `#[napi(string_enum)]` without an explicit case option emits the
46+
* Rust variant identifier as-is). We duplicate the values here instead
47+
* of importing `AuthMode` from `native/sea/index.d.ts` because that
48+
* file declares `AuthMode` as `export const enum`, which is
49+
* incompatible with `isolatedModules` and a runtime-coupling hazard.
50+
* The Rust source of truth lives at `native/sea/src/database.rs`.
5051
*/
5152
export type SeaNativeConnectionOptions =
5253
| {

lib/sea/SeaNativeLoader.ts

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
* forwards to the binding's `installLogger()` once that surface lands.
2828
*/
2929

30+
import type { SeaNativeConnectionOptions } from './SeaAuth';
31+
3032
// The path is relative to this file at runtime (`dist/sea/SeaNativeLoader.js`)
3133
// resolving to `dist/sea/../../native/sea/index.js` once `tsc` has emitted
3234
// to `dist/`. We use a require-time path resolution because the napi
@@ -52,21 +54,14 @@ export interface SeaNativeBinding {
5254
version(): string;
5355
/**
5456
* Open a session over PAT, OAuth M2M, or OAuth U2M auth. Returns an
55-
* opaque Connection. The discriminated payload mirrors the
56-
* auto-generated `ConnectionOptions` in `native/sea/index.d.ts`; we
57-
* keep the static type permissive (all fields optional except the
58-
* discriminant) so the JS adapter layer's union narrows correctly
59-
* without three overloads.
57+
* opaque Connection. The discriminated `SeaNativeConnectionOptions`
58+
* union from `SeaAuth` is the source of truth for the per-mode
59+
* required fields, so the loader-seam enforces the same compile-time
60+
* check the adapter applies — a caller can't bypass
61+
* `buildSeaConnectionOptions` and pass, say, `{ authMode: 'Pat' }`
62+
* with no token.
6063
*/
61-
openSession(opts: {
62-
hostName: string;
63-
httpPath: string;
64-
authMode: 'Pat' | 'OAuthM2m' | 'OAuthU2m';
65-
token?: string;
66-
oauthClientId?: string;
67-
oauthClientSecret?: string;
68-
oauthRedirectPort?: number;
69-
}): Promise<unknown>;
64+
openSession(opts: SeaNativeConnectionOptions): Promise<unknown>;
7065
/** Opaque Connection class — instance methods on the binding-generated d.ts. */
7166
Connection: Function;
7267
/** Opaque Statement class — instance methods on the binding-generated d.ts. */

native/sea/index.d.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,22 @@ export interface ExecuteOptions {
2222
/**
2323
* JS-visible auth-mode discriminant.
2424
*
25-
* Crosses the FFI as the string value (napi-rs string-enums emit the
26-
* Rust variant name verbatim on the JS side — `'Pat'`, `'OAuthM2m'`,
27-
* `'OAuthU2m'`). Keeping the discriminant explicit instead of inferring
28-
* it from "which Option is set" makes the napi-side validation
29-
* single-pass and the JS-side schema typed.
25+
* Crosses the FFI as the variant name verbatim — napi-rs's
26+
* `#[napi(string_enum)]` without an explicit case option emits the
27+
* Rust variant identifier as-is, so this enum becomes
28+
* `AuthMode.Pat = 'Pat'` / `AuthMode.OAuthM2m = 'OAuthM2m'` /
29+
* `AuthMode.OAuthU2m = 'OAuthU2m'` in the auto-generated
30+
* `native/sea/index.d.ts`. The JS adapter
31+
* (`SeaNativeConnectionOptions` in `lib/sea/SeaAuth.ts`) must use the
32+
* PascalCase literals verbatim.
33+
*
34+
* Keeping the discriminant explicit instead of inferring it from
35+
* "which Option is set" makes the napi-side validation single-pass
36+
* and the JS-side schema typed.
37+
*
38+
* Note: adding a variant here requires extending `open_session()`'s
39+
* `match` — Rust will fail the build if the match is non-exhaustive,
40+
* but the cross-reference shortens the review loop.
3041
*/
3142
export const enum AuthMode {
3243
Pat = 'Pat',

native/sea/src/database.rs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,22 @@ use crate::util::guarded;
4040

4141
/// JS-visible auth-mode discriminant.
4242
///
43-
/// Crosses the FFI as the string value (napi-rs string-enums emit the
44-
/// Rust variant name verbatim on the JS side — `'Pat'`, `'OAuthM2m'`,
45-
/// `'OAuthU2m'`). Keeping the discriminant explicit instead of inferring
46-
/// it from "which Option is set" makes the napi-side validation
47-
/// single-pass and the JS-side schema typed.
43+
/// Crosses the FFI as the variant name verbatim — napi-rs's
44+
/// `#[napi(string_enum)]` without an explicit case option emits the
45+
/// Rust variant identifier as-is, so this enum becomes
46+
/// `AuthMode.Pat = 'Pat'` / `AuthMode.OAuthM2m = 'OAuthM2m'` /
47+
/// `AuthMode.OAuthU2m = 'OAuthU2m'` in the auto-generated
48+
/// `native/sea/index.d.ts`. The JS adapter
49+
/// (`SeaNativeConnectionOptions` in `lib/sea/SeaAuth.ts`) must use the
50+
/// PascalCase literals verbatim.
51+
///
52+
/// Keeping the discriminant explicit instead of inferring it from
53+
/// "which Option is set" makes the napi-side validation single-pass
54+
/// and the JS-side schema typed.
55+
///
56+
/// Note: adding a variant here requires extending `open_session()`'s
57+
/// `match` — Rust will fail the build if the match is non-exhaustive,
58+
/// but the cross-reference shortens the review loop.
4859
#[napi(string_enum)]
4960
pub enum AuthMode {
5061
Pat,
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Copyright (c) 2026 Databricks, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
import { SeaNativeBinding } from '../../../../lib/sea/SeaNativeLoader';
16+
17+
export interface RecordedCall {
18+
method: string;
19+
args: unknown[];
20+
}
21+
22+
export interface FakeBinding {
23+
binding: SeaNativeBinding;
24+
calls: RecordedCall[];
25+
}
26+
27+
/**
28+
* Build a fake `SeaNativeBinding` that records every `openSession` call
29+
* and returns a `Connection` whose `close()` is also recorded. Shared
30+
* across the SEA auth unit-test files (PAT / M2M / U2M / future flows)
31+
* so the closure shape lives in exactly one place.
32+
*
33+
* No real native code runs — the fake is structural-typing-only.
34+
*/
35+
export function makeFakeBinding(): FakeBinding {
36+
const calls: RecordedCall[] = [];
37+
38+
const fakeConnection = {
39+
async executeStatement() {
40+
throw new Error('not used in this test');
41+
},
42+
async close() {
43+
calls.push({ method: 'connection.close', args: [] });
44+
},
45+
};
46+
47+
const binding: SeaNativeBinding = {
48+
version() {
49+
return 'fake-binding';
50+
},
51+
async openSession(opts: Parameters<SeaNativeBinding['openSession']>[0]) {
52+
calls.push({ method: 'openSession', args: [opts] });
53+
return fakeConnection as unknown;
54+
},
55+
Connection: function FakeConnection() {} as unknown as Function,
56+
Statement: function FakeStatement() {} as unknown as Function,
57+
};
58+
59+
return { binding, calls };
60+
}

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

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -15,37 +15,10 @@
1515
import { expect } from 'chai';
1616
import SeaBackend from '../../../lib/sea/SeaBackend';
1717
import { buildSeaConnectionOptions } from '../../../lib/sea/SeaAuth';
18-
import { SeaNativeBinding } from '../../../lib/sea/SeaNativeLoader';
1918
import { ConnectionOptions } from '../../../lib/contracts/IDBSQLClient';
2019
import AuthenticationError from '../../../lib/errors/AuthenticationError';
2120
import HiveDriverError from '../../../lib/errors/HiveDriverError';
22-
23-
function makeFakeBinding() {
24-
const calls: Array<{ method: string; args: unknown[] }> = [];
25-
26-
const fakeConnection = {
27-
async executeStatement() {
28-
throw new Error('not used in this test');
29-
},
30-
async close() {
31-
calls.push({ method: 'connection.close', args: [] });
32-
},
33-
};
34-
35-
const binding: SeaNativeBinding = {
36-
version() {
37-
return 'fake-binding';
38-
},
39-
async openSession(opts: Parameters<SeaNativeBinding['openSession']>[0]) {
40-
calls.push({ method: 'openSession', args: [opts] });
41-
return fakeConnection as unknown;
42-
},
43-
Connection: function FakeConnection() {} as unknown as Function,
44-
Statement: function FakeStatement() {} as unknown as Function,
45-
};
46-
47-
return { binding, calls };
48-
}
21+
import { makeFakeBinding } from './_helpers/fakeBinding';
4922

5023
describe('SeaAuth + SeaBackend — OAuth M2M auth flow', () => {
5124
describe('buildSeaConnectionOptions', () => {

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

Lines changed: 3 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -15,42 +15,10 @@
1515
import { expect } from 'chai';
1616
import SeaBackend from '../../../lib/sea/SeaBackend';
1717
import { buildSeaConnectionOptions } from '../../../lib/sea/SeaAuth';
18-
import { SeaNativeBinding } from '../../../lib/sea/SeaNativeLoader';
1918
import { ConnectionOptions } from '../../../lib/contracts/IDBSQLClient';
2019
import AuthenticationError from '../../../lib/errors/AuthenticationError';
2120
import HiveDriverError from '../../../lib/errors/HiveDriverError';
22-
23-
/**
24-
* Fake napi binding that records the option object handed to `openSession`
25-
* and returns a fake `Connection` whose `close()` we can observe. No real
26-
* native code runs in this suite.
27-
*/
28-
function makeFakeBinding() {
29-
const calls: Array<{ method: string; args: unknown[] }> = [];
30-
31-
const fakeConnection = {
32-
async executeStatement() {
33-
throw new Error('not used in this test');
34-
},
35-
async close() {
36-
calls.push({ method: 'connection.close', args: [] });
37-
},
38-
};
39-
40-
const binding: SeaNativeBinding = {
41-
version() {
42-
return 'fake-binding';
43-
},
44-
async openSession(opts: Parameters<SeaNativeBinding['openSession']>[0]) {
45-
calls.push({ method: 'openSession', args: [opts] });
46-
return fakeConnection as unknown;
47-
},
48-
Connection: function FakeConnection() {} as unknown as Function,
49-
Statement: function FakeStatement() {} as unknown as Function,
50-
};
51-
52-
return { binding, calls };
53-
}
21+
import { makeFakeBinding } from './_helpers/fakeBinding';
5422

5523
describe('SeaAuth + SeaBackend — PAT auth flow', () => {
5624
describe('buildSeaConnectionOptions', () => {
@@ -133,10 +101,8 @@ describe('SeaAuth + SeaBackend — PAT auth flow', () => {
133101
host: 'example.cloud.databricks.com',
134102
path: '/sql/1.0/warehouses/abc',
135103
authType: 'token-provider',
136-
tokenProvider: { getToken: async () => 'tok' } as unknown as ConnectionOptions extends infer T
137-
? // eslint-disable-next-line @typescript-eslint/no-explicit-any
138-
any
139-
: never,
104+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
105+
tokenProvider: { getToken: async () => 'tok' } as any,
140106
};
141107

142108
expect(() => buildSeaConnectionOptions(opts)).to.throw(

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

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -15,36 +15,9 @@
1515
import { expect } from 'chai';
1616
import SeaBackend from '../../../lib/sea/SeaBackend';
1717
import { buildSeaConnectionOptions } from '../../../lib/sea/SeaAuth';
18-
import { SeaNativeBinding } from '../../../lib/sea/SeaNativeLoader';
1918
import { ConnectionOptions } from '../../../lib/contracts/IDBSQLClient';
2019
import HiveDriverError from '../../../lib/errors/HiveDriverError';
21-
22-
function makeFakeBinding() {
23-
const calls: Array<{ method: string; args: unknown[] }> = [];
24-
25-
const fakeConnection = {
26-
async executeStatement() {
27-
throw new Error('not used in this test');
28-
},
29-
async close() {
30-
calls.push({ method: 'connection.close', args: [] });
31-
},
32-
};
33-
34-
const binding: SeaNativeBinding = {
35-
version() {
36-
return 'fake-binding';
37-
},
38-
async openSession(opts: Parameters<SeaNativeBinding['openSession']>[0]) {
39-
calls.push({ method: 'openSession', args: [opts] });
40-
return fakeConnection as unknown;
41-
},
42-
Connection: function FakeConnection() {} as unknown as Function,
43-
Statement: function FakeStatement() {} as unknown as Function,
44-
};
45-
46-
return { binding, calls };
47-
}
20+
import { makeFakeBinding } from './_helpers/fakeBinding';
4821

4922
describe('SeaAuth + SeaBackend — OAuth U2M auth flow', () => {
5023
describe('buildSeaConnectionOptions', () => {

0 commit comments

Comments
 (0)